feat: flat storage cache #8540

Longarithm · 2023-02-08T20:33:55Z

Based on @jakmeier' estimations, we need to cache ValueRefs for flat storage head (see #8006). RocksDB internal impl and block cache doesn't help, and we need to make flat storage performance to be at least comparable to trie performance in MVP, in order not to make undercharging issue worse.

This cache lives inside FlatStorageState, can be accessed in get_ref before attempt to read value ref from flat storage head, and must be updated when we apply delta.

I think it makes sense to make cache capacity configurable, and this config fits into StoreConfig. I don't like that is propagated to FlatStorageState from ShardTries, it makes trie storage and flat storage mixed even more. Perhaps it needs to be fully moved inside FlatStateFactory, but I am not sure.

Testing

extend flat_storage_state_sanity to check both cached and non-cached versions;
flat_storage_state_cache_eviction to check that eviction strategy is applied correctly.

jakmeier · 2023-02-09T09:53:46Z

@Longarithm I discussed with @akhi3030 that it seems a bit odd we need to build this cache on top of RocksDB, when RocksDB allows for efficient caching. I would like to test if enabling the RowCache (a cache that stores key-values pairs instead of blocks but it is currently disabled for all columns) in RocksDB couldn't achieve the same goal. I think it would make the code and its maintenance easier.

But it will take me at least a week to get this benchmarked based on current priorities. What do you think, should we wait with this PR until we know the results?

Longarithm · 2023-02-09T11:10:13Z

It's funny that when I googled RowCache, this was the first link https://groups.google.com/g/rocksdb/c/YxdRryNVTyw/m/ZobfhrO8AAAJ

After reading code I found that rocksdb has RowCache, but it seems to be not widely spoken, not even in rocksdb wiki, wierd, Anyone got clue?

If it actually helps, that would be great, I'm interested in new results. If I remember correctly, RocksDB block cache didn't help with that.
I'll still use this custom cache in my runs though.

wacban · 2023-02-09T11:34:44Z

core/store/src/flat_state.rs

+                if guard.value_ref_cache.len() == guard.value_ref_cache.cap() {
+                    if let Some((key, _)) = guard.value_ref_cache.pop_lru() {
+                        guard.metrics.value_ref_cache_total_key_size.sub(key.len() as i64);
+                    }
+                }
+                if guard.value_ref_cache.len() < guard.value_ref_cache.cap() {
+                    guard.metrics.value_ref_cache_total_key_size.add(key.len() as i64);
+                    guard.value_ref_cache.put(key.clone(), value.clone());
+                }


You may be able to simplify this code with the lru cache push method - it checks the capacity by itself and returns a popped item if any. Not sure how it would handle 0 capacity though.

https://docs.rs/lru/latest/lru/struct.LruCache.html#method.push

wacban · 2023-02-09T11:36:20Z

core/store/src/metrics.rs

+pub static FLAT_STORAGE_VALUE_REF_CACHE_TOTAL_KEY_SIZE: Lazy<IntGaugeVec> = Lazy::new(|| {
+    try_create_int_gauge_vec(
+        "flat_storage_value_ref_cache_total_key_size",
+        "Total size of all keys in flat storage cache for its head",


Out of curiosity why do you only measure the size of the keys and not keys and values?

Because size of values is fixed - we store ValueRefs instead of values.

Should we measure both since we might implement inlining in the future?

wacban · 2023-02-09T11:38:54Z

core/store/src/flat_state.rs

@@ -991,6 +1011,9 @@ impl FlatStorageState {
            };
        }

+        if let Some(value_ref) = guard.get_cached_ref(key) {
+            return Ok(value_ref);
+        }
        Ok(store_helper::get_ref(&guard.store, key)?)


Should you also push the just read value back to the cache? Currently you only update values in the cache when writing new values there. I suppose either can be fine but cache hit rate will heavily depend on the usage pattern.

jakmeier · 2023-02-09T11:58:08Z

To understand the row cache, I found this blog post (from January this year) helpful: https://betterprogramming.pub/navigating-the-minefield-of-rocksdb-configuration-options-246af1e1d3f9

Especially this part:

Row cache
This cache is used for storing actual data for a key. The cache’s documentation is almost non-existent, but it provides fast data access in case you do a lot of point lookups. RocksDB first looks in the row cache and then in the block cache for the data. Like block cache, you can use either LRUCache or ClockCache as the actual implementation for row cache.

wacban · 2023-02-10T12:31:57Z

lgtm but I'll let someone familiar with flat storage review it too
might be cool to add some cache hit rate metrics too if this is something you want to track

mzhangmzz · 2023-02-11T00:09:42Z

core/store/src/flat_state.rs

+
+    /// Get cached `ValueRef` for flat storage head.
+    #[cfg(feature = "protocol_feature_flat_state")]
+    fn get_cached_ref(&mut self, key: &[u8]) -> Option<Option<ValueRef>> {


Explain why the returned value is Option<Option<>> since it could be confusing.

mzhangmzz

LGTM. Previously I implemented an empty struct called FlatStateCache with the intention of adding cache implementation and it is part of FlatState. Could you remove that code? You can either do it in this PR or in a separate PR. Thank you!

mzhangmzz · 2023-02-11T00:12:56Z

core/store/src/metrics.rs

+pub static FLAT_STORAGE_VALUE_REF_CACHE_TOTAL_KEY_SIZE: Lazy<IntGaugeVec> = Lazy::new(|| {
+    try_create_int_gauge_vec(
+        "flat_storage_value_ref_cache_total_key_size",
+        "Total size of all keys in flat storage cache for its head",


Should we measure both since we might implement inlining in the future?

Longarithm · 2023-02-27T20:38:36Z

We don't have time for researching RocksDB RowCache for now, added todo to the code: #8649

Part of #8577 and #8684, was introduced in #8540. This PR removes custom `FlatState` cache from `FlatStorage` since experiments show very low hit rate which is not worth used memory.

Part of near#8577 and near#8684, was introduced in near#8540. This PR removes custom `FlatState` cache from `FlatStorage` since experiments show very low hit rate which is not worth used memory.

Looogarithm added 3 commits February 8, 2023 23:55

cache

a76f009

propagate cfg parameter

1e7cf25

metrics

0c9d93d

Longarithm self-assigned this Feb 8, 2023

Longarithm requested review from pugachAG, mzhangmzz and jakmeier February 8, 2023 20:48

Merge branch 'master' into fs-cache

f2f3007

Longarithm marked this pull request as ready for review February 8, 2023 20:49

Longarithm requested a review from a team as a code owner February 8, 2023 20:49

wacban reviewed Feb 9, 2023

View reviewed changes

fix eviction policy

cec18d5

mzhangmzz reviewed Feb 11, 2023

View reviewed changes

mzhangmzz approved these changes Feb 11, 2023

View reviewed changes

Looogarithm added 7 commits February 27, 2023 22:43

Merge branch 'master' into fs-cache

efc6e73

small cache cap

9475d1b

add get_ref

3d33667

add back

3dfff45

add back

5fd9d61

fix value

125ce24

fix value

369bcff

Longarithm added the S-automerge label Feb 27, 2023

add todo

339be5f

near-bulldozer bot added 2 commits February 28, 2023 07:23

Merge refs/heads/master into fs-cache

e51336c

Merge refs/heads/master into fs-cache

c2f2975

near-bulldozer bot and others added 4 commits February 28, 2023 12:58

Merge refs/heads/master into fs-cache

7fe6e57

fmt

1f7e47e

Merge branch 'fs-cache' of github.com:near/nearcore into fs-cache

b10d433

fix clippy error

942c66a

near-bulldozer bot merged commit 229e6ae into master Feb 28, 2023

near-bulldozer bot deleted the fs-cache branch February 28, 2023 13:41

jakmeier mentioned this pull request Feb 28, 2023

Flat State Gas Costs (Read FS only) #8006

Closed

6 tasks

pugachAG mentioned this pull request Mar 14, 2023

feat: remove flat state cache #8727

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: flat storage cache #8540

feat: flat storage cache #8540

Longarithm commented Feb 8, 2023 •

edited

Loading

jakmeier commented Feb 9, 2023

Longarithm commented Feb 9, 2023

wacban Feb 9, 2023

wacban Feb 9, 2023

Longarithm Feb 9, 2023

mzhangmzz Feb 11, 2023

wacban Feb 9, 2023

Longarithm Feb 9, 2023

jakmeier commented Feb 9, 2023

wacban commented Feb 10, 2023

mzhangmzz Feb 11, 2023

mzhangmzz left a comment

mzhangmzz Feb 11, 2023

Longarithm commented Feb 27, 2023

feat: flat storage cache #8540

feat: flat storage cache #8540

Conversation

Longarithm commented Feb 8, 2023 • edited Loading

Testing

jakmeier commented Feb 9, 2023

Longarithm commented Feb 9, 2023

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

jakmeier commented Feb 9, 2023

wacban commented Feb 10, 2023

Choose a reason for hiding this comment

mzhangmzz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Longarithm commented Feb 27, 2023

Longarithm commented Feb 8, 2023 •

edited

Loading