feat(storage): Trie cache limit on value cached #2770
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Don't cache large values and assume that accessing rocksdb is good enough for them. Also don't cache when a hash is missing from storage: retrieve_rc does not benefit from caching those cases, and retrieve_raw_bytes is never supposed to be called for missing values. Also add no_cache feature to near-store and make runtime-params-estimator enable it. Fixes #940 Test plan --------- Existing tests
mikhailOK
requested review from
evgenykuzyakov,
frol,
olonho and
SkidanovAlex
as code owners
|
|
MaksymZavershynskyi
approved these changes
Jun 2, 2020
core/store/src/trie/trie_storage.rs
Outdated
|
|
||
| /// Values above this size are never cached. | ||
| /// Note that Trie inner nodes are always smaller than this. | ||
| const TRIE_MAX_VALUE_CACHED: usize = 4000; |
There was a problem hiding this comment.
It's better to annotate the unit as well (looks like it's byte here).
There was a problem hiding this comment.
I suggest we call it TRIE_LIMIT_CACHED_VALUE_SIZE; I find the current name confusing as it sounds like there are values 1, 2, 3, .... 4000, 100500, and we only cache the values that are below 4000 (max value).
bowenwang1996
approved these changes
Jun 3, 2020
frol
approved these changes
Jun 3, 2020
core/store/src/trie/trie_storage.rs
Outdated
|
|
||
| /// Values above this size are never cached. | ||
| /// Note that Trie inner nodes are always smaller than this. | ||
| const TRIE_MAX_VALUE_CACHED: usize = 4000; |
There was a problem hiding this comment.
I suggest we call it TRIE_LIMIT_CACHED_VALUE_SIZE; I find the current name confusing as it sounds like there are values 1, 2, 3, .... 4000, 100500, and we only cache the values that are below 4000 (max value).
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Don't cache large values and assume that accessing rocksdb is good
enough for them.
Also don't cache when a hash is missing from storage: retrieve_rc does
not benefit from caching those cases, and retrieve_raw_bytes is never
supposed to be called for missing values.
Also add no_cache feature to near-store and make
runtime-params-estimator enable it.
Fixes #940
Test plan
Existing tests