EN-11730: use a context + predefined channel for get all leaves operations #3900

bogdan-rosianu · 2022-03-14T14:07:12Z

Refactor GetAllLeaves / GetAllLeavesOnChannel functions so they receive the channel instead of creating it + they receive a context as well that can trigger an early exit on the processing.

Implemented a timeout mechanism for node's API endpoints that called that function

…tions

codecov · 2022-03-15T08:51:42Z

Codecov Report

Merging #3900 (0d9245d) into development (a35ed26) will increase coverage by 0.00%.
The diff coverage is 81.88%.

@@             Coverage Diff              @@
##           development    #3900   +/-   ##
============================================
  Coverage        74.88%   74.88%           
============================================
  Files              614      614           
  Lines            81804    81864   +60     
============================================
+ Hits             61255    61305   +50     
- Misses           15889    15900   +11     
+ Partials          4660     4659    -1

Impacted Files	Coverage Δ
factory/processComponents.go	`64.81% <0.00%> (-0.07%)`	⬇️
trie/leafNode.go	`79.04% <0.00%> (-0.89%)`	⬇️
trie/branchNode.go	`78.81% <20.00%> (-0.45%)`	⬇️
trie/extensionNode.go	`71.39% <20.00%> (-0.51%)`	⬇️
trie/patriciaMerkleTrie.go	`73.96% <80.00%> (ø)`
facade/nodeFacade.go	`68.88% <81.81%> (+1.27%)`	⬆️
node/node.go	`72.77% <92.00%> (+0.20%)`	⬆️
epochStart/metachain/systemSCs.go	`71.59% <100.00%> (+0.03%)`	⬆️
node/external/nodeApiResolver.go	`59.79% <100.00%> (ø)`
node/trieIterators/delegatedListProcessor.go	`85.18% <100.00%> (+4.23%)`	⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c563557...0d9245d. Read the comment docs.

iulianpascalau · 2022-03-15T15:15:24Z

epochStart/metachain/systemSCs.go

@@ -1128,7 +1129,8 @@ func (s *systemSCProcessor) getArgumentsForSetOwnerFunctionality(userValidatorAc
 		return nil, err
 	}

-	chLeaves, err := userValidatorAccount.DataTrie().GetAllLeavesOnChannel(rootHash)
+	chLeaves := make(chan core.KeyValueHolder, common.TrieLeavesChannelDefaultCapacity)
+	err = userValidatorAccount.DataTrie().GetAllLeavesOnChannel(chLeaves, context.Background(), rootHash)


👍
context.Background() is correct in this situation

iulianpascalau · 2022-03-15T15:19:08Z

factory/processComponents.go

@@ -781,7 +782,8 @@ func (pcf *processComponentsFactory) indexGenesisAccounts() error {
 		return err
 	}

-	leavesChannel, err := pcf.state.AccountsAdapter().GetAllLeaves(rootHash)
+	leavesChannel := make(chan core.KeyValueHolder, common.TrieLeavesChannelDefaultCapacity)
+	err = pcf.state.AccountsAdapter().GetAllLeaves(leavesChannel, context.Background(), rootHash)


👍
context.Background() is correct in this situation

iulianpascalau · 2022-03-15T15:19:20Z

go.mod

iulianpascalau · 2022-03-15T15:34:46Z

node/trieIterators/delegatedListProcessor.go

@@ -124,7 +126,8 @@ func (dlp *delegatedListProcessor) getDelegatorsList(delegationSC []byte) ([][]b
 		return nil, fmt.Errorf("%w for delegationSC %s", err, hex.EncodeToString(delegationSC))
 	}

-	chLeaves, err := delegatorAccount.DataTrie().GetAllLeavesOnChannel(rootHash)
+	chLeaves := make(chan core.KeyValueHolder, common.TrieLeavesChannelDefaultCapacity)
+	err = delegatorAccount.DataTrie().GetAllLeavesOnChannel(chLeaves, context.Background(), rootHash)


could have injected a context here

iulianpascalau · 2022-03-15T15:40:26Z

node/trieIterators/delegatedListProcessor_test.go

-				close(ch)
-			}()
+					close(ch)
+				}()


indenting gone wild?

iulianpascalau · 2022-03-15T16:17:07Z

process/peer/process.go

@@ -572,7 +573,8 @@ func (vs *validatorStatistics) GetValidatorInfoForRootHash(rootHash []byte) (map
 		log.Debug("GetValidatorInfoForRootHash", sw.GetMeasurements()...)
 	}()

-	leavesChannel, err := vs.peerAdapter.GetAllLeaves(rootHash)
+	leavesChannel := make(chan core.KeyValueHolder, common.TrieLeavesChannelDefaultCapacity)
+	err := vs.peerAdapter.GetAllLeaves(leavesChannel, context.Background(), rootHash)


👍
context.Background() is correct in this situation

iulianpascalau · 2022-03-15T16:18:22Z

state/accountsDB_test.go

-		GetAllLeavesOnChannelCalled: func(rootHash []byte) (chan core.KeyValueHolder, error) {
-			getAllLeavesCalled = true
+		GetAllLeavesOnChannelCalled: func(ch chan core.KeyValueHolder, ctx context.Context, rootHash []byte) error {
+				getAllLeavesCalled = true


indentation gone wild?

iulianpascalau · 2022-03-15T16:18:56Z

state/syncer/userAccountsSyncer.go

@@ -211,7 +211,8 @@ func (u *userAccountsSyncer) syncAccountDataTries(
 		return err
 	}

-	leavesChannel, err := mainTrie.GetAllLeavesOnChannel(mainRootHash)
+	leavesChannel := make(chan core.KeyValueHolder, common.TrieLeavesChannelDefaultCapacity)
+	err = mainTrie.GetAllLeavesOnChannel(leavesChannel, context.Background(), mainRootHash)


👍
context.Background() is correct in this situation

iulianpascalau · 2022-03-15T16:20:21Z

trie/errors.go

@@ -105,3 +105,6 @@ var ErrKeyNotFound = errors.New("key not found")

 // ErrNilEpochNotifier signals that the provided EpochNotifier is nil
 var ErrNilEpochNotifier = errors.New("nil EpochNotifier")
+
+// ErrContextDeadlineExceeded signals that a context has exceeded it's deadline
+var ErrContextDeadlineExceeded = errors.New("context deadline exceeded")


this is unused

iulianpascalau · 2022-03-15T16:22:45Z

update/genesis/export.go

@@ -269,7 +270,8 @@ func (se *stateExport) exportTrie(key string, trie common.Trie) error {
 		return err
 	}

-	leavesChannel, err := trie.GetAllLeavesOnChannel(rootHash)
+	leavesChannel := make(chan core.KeyValueHolder, common.TrieLeavesChannelDefaultCapacity)
+	err = trie.GetAllLeavesOnChannel(leavesChannel, context.Background(), rootHash)


👍
context.Background() is correct in this situation

gabi-vuls

System test passed.

… into EN-11730-get-all-leaves-context

facade/nodeFacade.go

gabi-vuls

System test passed

… into EN-11730-get-all-leaves-context

gabi-vuls

System test passed.

bogdan-rosianu and others added 5 commits March 14, 2022 16:06

EN-11730: use a context + predefined channel for get all leaves opera…

2433a5b

…tions

fix tests

b25aaf7

further tests fixes

cf572e9

fix race condition

a18fa36

Merge branch 'development' into EN-11730-get-all-leaves-context

6ea1583

integrate ctx with timeout on api trie range endpoints

20d11dd

bogdan-rosianu marked this pull request as ready for review March 15, 2022 15:09

iulianpascalau self-requested a review March 15, 2022 15:10

iulianpascalau reviewed Mar 15, 2022

View reviewed changes

bogdan-rosianu self-assigned this Mar 15, 2022

Merge branch 'development' into EN-11730-get-all-leaves-context

df70402

gabi-vuls previously approved these changes Mar 16, 2022

View reviewed changes

bogdan-rosianu added 2 commits March 16, 2022 12:01

EN-11730: fixes after review

ccb8834

Merge remote-tracking branch 'origin/EN-11730-get-all-leaves-context'…

543c385

… into EN-11730-get-all-leaves-context

bogdan-rosianu dismissed gabi-vuls’s stale review via 543c385 March 16, 2022 10:23

miiu96 self-requested a review March 16, 2022 10:48

miiu96 reviewed Mar 16, 2022

View reviewed changes

facade/nodeFacade.go Outdated Show resolved Hide resolved

fix after review

71b648a

iulianpascalau previously approved these changes Mar 17, 2022

View reviewed changes

gabi-vuls previously approved these changes Mar 17, 2022

View reviewed changes

Merge branch 'development' into EN-11730-get-all-leaves-context

8744ac9

miiu96 previously approved these changes Mar 18, 2022

View reviewed changes

iulianpascalau and others added 3 commits March 18, 2022 13:08

Merge branch 'development' into EN-11730-get-all-leaves-context

6a59d64

increased sleeps for ctx tests

8c5a484

Merge remote-tracking branch 'origin/EN-11730-get-all-leaves-context'…

e43a33d

… into EN-11730-get-all-leaves-context

bogdan-rosianu dismissed stale reviews from miiu96, gabi-vuls, and iulianpascalau via e43a33d March 18, 2022 11:37

Merge branch 'development' into EN-11730-get-all-leaves-context

0d9245d

iulianpascalau approved these changes Mar 18, 2022

View reviewed changes

miiu96 approved these changes Mar 18, 2022

View reviewed changes

gabi-vuls approved these changes Mar 18, 2022

View reviewed changes

gabi-vuls merged commit d5b958a into development Mar 18, 2022

gabi-vuls deleted the EN-11730-get-all-leaves-context branch March 18, 2022 13:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EN-11730: use a context + predefined channel for get all leaves operations #3900

EN-11730: use a context + predefined channel for get all leaves operations #3900

bogdan-rosianu commented Mar 14, 2022 •

edited

codecov bot commented Mar 15, 2022 •

edited

iulianpascalau Mar 15, 2022

iulianpascalau Mar 15, 2022

iulianpascalau Mar 15, 2022

iulianpascalau Mar 15, 2022

bogdan-rosianu Mar 16, 2022

iulianpascalau Mar 15, 2022

bogdan-rosianu Mar 16, 2022

iulianpascalau Mar 15, 2022

iulianpascalau Mar 15, 2022

bogdan-rosianu Mar 16, 2022

iulianpascalau Mar 15, 2022

iulianpascalau Mar 15, 2022

bogdan-rosianu Mar 16, 2022

iulianpascalau Mar 15, 2022

gabi-vuls left a comment

gabi-vuls left a comment

gabi-vuls left a comment

EN-11730: use a context + predefined channel for get all leaves operations #3900

EN-11730: use a context + predefined channel for get all leaves operations #3900

Conversation

bogdan-rosianu commented Mar 14, 2022 • edited

codecov bot commented Mar 15, 2022 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabi-vuls left a comment

Choose a reason for hiding this comment

gabi-vuls left a comment

Choose a reason for hiding this comment

gabi-vuls left a comment

Choose a reason for hiding this comment

bogdan-rosianu commented Mar 14, 2022 •

edited

codecov bot commented Mar 15, 2022 •

edited