tree: Add ApplyBatch method #1449

Merged
fyrchik merged 2 commits from dstepanov-yadro/frostfs-node:fix/tree_sync_race into master 2024-11-12 12:11:08 +00:00

Concurrent Apply can lead to child node applies before parent, so
undo/redo operations will perform. This leads to performance degradation
in case of tree with many sublevels.

Concurrent Apply can lead to child node applies before parent, so undo/redo operations will perform. This leads to performance degradation in case of tree with many sublevels.
dstepanov-yadro added 1 commit 2024-10-25 08:17:44 +00:00
[#9999] tree: Add ApplyBatch method
Some checks failed
DCO action / DCO (pull_request) Successful in 1m6s
Tests and linters / Run gofumpt (pull_request) Successful in 1m29s
Vulncheck / Vulncheck (pull_request) Successful in 2m3s
Pre-commit hooks / Pre-commit (pull_request) Successful in 2m10s
Tests and linters / Lint (pull_request) Failing after 2m31s
Build / Build Components (pull_request) Successful in 2m37s
Tests and linters / gopls check (pull_request) Successful in 2m45s
Tests and linters / Staticcheck (pull_request) Successful in 3m7s
Tests and linters / Tests (pull_request) Successful in 4m35s
Tests and linters / Tests with -race (pull_request) Successful in 5m44s
572da86df7
Concurrent Apply can lead to child node applies before parent, so
undo/redo operations will perform. This leads to performance degradation
in case of tree with many sublevels.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
dstepanov-yadro force-pushed fix/tree_sync_race from 572da86df7 to 5dd4f0afd7 2024-10-25 08:43:10 +00:00 Compare
dstepanov-yadro force-pushed fix/tree_sync_race from 5dd4f0afd7 to 87ce7722c7 2024-10-25 12:54:49 +00:00 Compare
fyrchik reviewed 2024-11-11 06:54:02 +00:00
@ -384,7 +376,7 @@ func (s *Service) syncLoop(ctx context.Context) {
return
case <-s.syncChan:
ctx, span := tracing.StartSpanFromContext(ctx, "TreeService.sync")
s.log.Debug(logs.TreeSyncingTrees)
Owner

Should be in a separate commit.

Should be in a separate commit.
Author
Member

done

done
fyrchik marked this conversation as resolved
@ -202,3 +190,1 @@
unappliedOperationHeight = min(unappliedOperationHeight, m.Time)
heightMtx.Unlock()
return err
if len(batch) == 1000 {
Owner

Magic constant.

Magic constant.
Author
Member

fixed

fixed
fyrchik marked this conversation as resolved
dstepanov-yadro force-pushed fix/tree_sync_race from 87ce7722c7 to dbc91a4570 2024-11-11 09:14:51 +00:00 Compare
dstepanov-yadro changed title from WIP: tree: Add ApplyBatch method to tree: Add ApplyBatch method 2024-11-11 09:21:15 +00:00
dstepanov-yadro requested review from storage-core-committers 2024-11-11 09:22:52 +00:00
dstepanov-yadro requested review from storage-core-developers 2024-11-11 09:22:53 +00:00
fyrchik approved these changes 2024-11-11 10:20:02 +00:00
Dismissed
acid-ant reviewed 2024-11-11 10:51:28 +00:00
@ -561,0 +609,4 @@
}
ops := make([]*Move, 0, len(m))
for _, op := range m {
Member

Can we move this loop inside View? In case of nil for treeRoot we may exit from it faster.

Can we move this loop inside `View`? In case of `nil` for `treeRoot` we may exit from it faster.
Author
Member

fixed

fixed
acid-ant reviewed 2024-11-11 10:52:50 +00:00
@ -106,6 +106,33 @@ func (s *Shard) TreeApply(ctx context.Context, cnr cidSDK.ID, treeID string, m *
return s.pilorama.TreeApply(ctx, cnr, treeID, m, backgroundSync)
}
// TreeApply implements the pilorama.Forest interface.
Member

TreeApply -> TreeApplyBatch

`TreeApply` -> `TreeApplyBatch`
Author
Member

fixed

fixed
dstepanov-yadro force-pushed fix/tree_sync_race from dbc91a4570 to 743e960b3b 2024-11-11 11:53:14 +00:00 Compare
dstepanov-yadro dismissed fyrchik's review 2024-11-11 11:53:14 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

acid-ant approved these changes 2024-11-11 12:12:32 +00:00
@ -561,0 +620,4 @@
var logKey [8]byte
binary.BigEndian.PutUint64(logKey[:], op.Time)
seen := b.Get(logKey[:]) != nil
if !seen {
Member

!seen -> b.Get(logKey[:]) == nil

`!seen` -> `b.Get(logKey[:]) == nil`
Author
Member

No, thx. With seen is clearer.

No, thx. With `seen` is clearer.
fyrchik approved these changes 2024-11-11 12:38:21 +00:00
fyrchik merged commit 46fef276b4 into master 2024-11-12 12:11:08 +00:00
fyrchik referenced this pull request from a commit 2024-11-12 12:11:09 +00:00
Sign in to join this conversation.
No reviewers
TrueCloudLab/storage-core-developers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#1449
No description provided.