En 8240 optimize get all leaves process #2490
Conversation
data/syncer/userAccountsSyncer.go
Outdated
| account := state.NewEmptyUserAccount() | ||
| err = u.marshalizer.Unmarshal(account, leaf) | ||
| err := u.marshalizer.Unmarshal(account, leaf.Value()) |
data/trie/patriciaMerkleTrie.go
Outdated
| tr.mutOperation.RUnlock() | ||
|
|
||
| go func() { | ||
| tr.mutOperation.RLock() | ||
| err := tr.root.getAllLeavesOnChannel(leavesChannel, []byte{}, tr.Database(), tr.marshalizer) | ||
| err = newTrie.root.getAllLeavesOnChannel(leavesChannel, []byte{}, tr.Database(), tr.marshalizer) |
There was a problem hiding this comment.
shouldn't you return this error as well ? like errFound and mutex ?
There was a problem hiding this comment.
Will live like this at the moment, like discussed.
data/state/accountsDB.go
Outdated
| @@ -767,22 +768,22 @@ func (adb *AccountsDB) recreateTrie(rootHash []byte) error { | |||
|
|
|||
| // RecreateAllTries recreates all the tries from the accounts DB | |||
| func (adb *AccountsDB) RecreateAllTries(rootHash []byte) (map[string]data.Trie, error) { | |||
| recreatedTrie, err := adb.mainTrie.Recreate(rootHash) | |||
| leavesChannel, err := adb.mainTrie.GetAllLeavesOnChannel(rootHash) | |||
There was a problem hiding this comment.
why did we switched the operations here?
There was a problem hiding this comment.
If GetAllLeavesOnChannel() returns an error, there will be no need for the recreatedTrie.
data/trie/patriciaMerkleTrie.go
Outdated
| if tr.root == nil { | ||
|
|
||
| newTrie, err := tr.recreate(rootHash) | ||
| if err != nil || check.IfNil(newTrie) || tr.root == nil { |
There was a problem hiding this comment.
Why do we need the last condition? It is creating a new trie anyway.
There was a problem hiding this comment.
Yes, but it can create a new empty trie, which has a nil root node.
There was a problem hiding this comment.
then, should have been: newTrie.root == nil ?
|
|
||
| for _, pa := range sliceLeaves { | ||
| peerAccount, err := vs.unmarshalPeer(pa) | ||
| for pa := range leavesChannel { |
There was a problem hiding this comment.
Interesting. This channel should cause deterministic outputs on each elrond-go node since the trie nodes traversal is deterministic. The sort you remove should no longer be necessary. However, we might get into backwards incompatibility issues. We have to check.
There was a problem hiding this comment.
Will check for backwards compatibility issues.
update/genesis/export.go
Outdated
| identifier string, | ||
| ) error { | ||
| for leaf := range leavesChannel { | ||
| keyToExport := CreateAccountKey(accType, shId, leaf.Key()) | ||
|
|
update/genesis/export.go
Outdated
| identifier string, | ||
| ) error { | ||
| for leaf := range leavesChannel { | ||
| keyToExport := CreateAccountKey(accType, shId, leaf.Key()) | ||
|
|
BeniaminDrasovean
dismissed stale reviews from iulianpascalau and sasurobert
via
c375ff9
Replaced GetAllLeaves() with GetAllLeavesOnChannel() throughout the whole project. The new implementation creates a new trie instance and runs getAllLeavesOnChannel(), meaning that once a node has been fully visited, it's children can be = nil. This way, no matter how big the state is, it will not fill the memory. By running getAllLeaves() on a new trie instance, it also means that the operation is not blocking, it just buffers the pruning operations. The pruning operations will be executed once there is no pruning blocking operation in progress (snapshot, getAllLeaves).