-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Make SharedTrieCache/LocalTrieCache work with entire state in memory #7682
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
AndreiEres
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can we measure the changes?
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
bkchr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looking good, mainly left some comments on smaller changes. One of the main things I don't want to see is that we expose this via CLI nor via a config option. I don't see why we should do this.
Co-authored-by: Bastian Köcher <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
|
All GitHub workflows were cancelled due to failure one of the required jobs. |
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
AndreiEres
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
Co-authored-by: Bastian Köcher <[email protected]>
Signed-off-by: Alexandru Gheorghe <[email protected]>
|
Changes are running on |
There weren't any problem while running on asset-hub-collators, merging this now. |
…7682) Part of #6131 (comment), we need to make sure that the TrieCache is able to work with the assumption that more or less the entire current state fits into memory, this can be split into a few parts: - [x] Remove the LocalTrieCache hard coded size and make it configurable from outside. - [x] Decided to have two flavours of building the LocalTrieCache, for the import and block authoring we are using a trusted local cache that grow to hold everything in the block and then propagated everything into the shared trie cache. - ~[x] Everything from LocalTrieCache needs to be propagated back to the SharedTrieCache in a fast and safe manner, so that it doesn't become a bottleneck. Currently, that is done on drop, with the function holding the ShareTrieCache write lock while promoting all keys. Decided to move this on a separate worker thread, so that the hot-path does not have to wait for drop function to propagate everything from the LocalTrieCache to the SharedTrieCache, the flushing of the worker thread happens after the block production and import happens.~ Update this is not need because authoring and imports is bounded, so the numbers is not exaggeratedly big, to make the drop too heavy, see numbers here #7682 (comment). # Todo - [x] Stats to prove most of reads/writes hit the shared or the local trie cache. - [x] Unit testing the new changes. Fixes: #7534 --------- Signed-off-by: Alexandru Gheorghe <[email protected]> Co-authored-by: Bastian Köcher <[email protected]>
Part of #6131 (comment), we need to make sure that the TrieCache is able to work with the assumption that more or less the entire current state fits into memory, this can be split into a few parts:
[x] Everything from LocalTrieCache needs to be propagated back to the SharedTrieCache in a fast and safe manner, so that it doesn't become a bottleneck. Currently, that is done on drop, with the function holding the ShareTrieCache write lock while promoting all keys. Decided to move this on a separate worker thread, so that the hot-path does not have to wait for drop function to propagate everything from the LocalTrieCache to the SharedTrieCache, the flushing of the worker thread happens after the block production and import happens.Update this is not need because authoring and imports is bounded, so the numbers is not exaggeratedly big, to make the drop too heavy, see numbers here Make SharedTrieCache/LocalTrieCache work with entire state in memory #7682 (comment).Todo
Fixes: #7534