feat: construct state sync parts using flat storage #8927

Longarithm · 2023-04-18T17:29:06Z

Gennerate inner part of state part using flat storage using idea present in #8984.

For reference - this makes state sync 100x faster, now SS for shard 3 should complete in 30 minutes :) https://near.zulipchat.com/#narrow/stream/297873-pagoda.2Fnode/topic/State.20Sync.20update.202023-05-23/near/360715229

In short, if flat storage head corresponds to the state root for which we sync state, it is enough to read only boundary nodes, and inner trie part can be reconstructed using range of KV pairs from state. The main logic for that is contained in Trie::get_trie_nodes_for_part_with_flat_storage.

It requires couple of minor changes:

now we allow creating "view" Tries with flat storage as well. As before, we want to avoid creating non-view Tries because TrieCache accesses may be blocking for chunk processing
get_head_hash and shard_uid methods for FlatStorage allowing to make correct range query to flat storage
FlatStateValue moved to primitives to allow more general access

TODO

prometheus metrics
integration test checking that flat storage is used during normal block processing on client (or wait for feat(state-sync): DB Snapshots #9090)

Testing

https://nayduck.near.org/#/run/3023

Big sanity test get_trie_nodes_for_part_with_flat_storage covering all scenarios I could think of:

results with/without flat storage must match
result with incorrect flat storage must be an error
result with flat storage and missing intermediate node should be still okay

@jbajic

Needed for #8927. 1. From what I see in https://doc.rust-lang.org/rust-by-example/scope/lifetime/explicit.html, lifetimes introduce restriction over passed variables - they must outlive function lifetime. But this doesn't make sense for iterator. It doesn't save much memory, because iterator is expected to return lots of vectors anyway. And slices `from` and `to` are converted to vectors inside anyway. This allows us to construct DB keys inside `iter_flat_state_entries`. 2. Construction of DB keys is hidden inside `flat` module now. You give state keys and receive state keys back, without knowing how they are stored on the lower level. ## Testing @jbajic would you like to cover `iter_flat_state_entries` with some test? This function will be covered by integration tests for state parts anyway and it is not used much currently. But it seems nice to have a test which creates flat storage for, say, 3 shards, writes some KV pairs to it and then checks that flat state iterator gives us correct data.

pugachAG

overall LGTM!

pugachAG · 2023-05-25T15:33:36Z

core/store/src/flat/manager.rs

+            match flat_storages.get(&shard_uid) {
+                Some(flat_storage) => flat_storage.clone(),
+                None => {
+                    debug!(target: "chain", "FlatStorage is not ready");


nit: I guess "store" target would be more appropriate than "chain" here

pugachAG · 2023-05-25T15:35:50Z

core/store/src/flat/store_helper.rs

-    shard_layout: &ShardLayout,
-    shard_id: u64,
-) -> Result<bool, StorageError> {
+pub fn key_belongs_to_shard(key: &Box<[u8]>, shard_uid: &ShardUId) -> Result<bool, StorageError> {


let's fix key type since we are changing this code: key: &[u8]

pugachAG · 2023-05-25T15:43:01Z

core/store/src/trie/shard_tries.rs

-            self.0.flat_storage_manager.chunk_view(shard_uid, block_hash, is_view);
+        let flat_storage_chunk_view = block_hash
+            .map(|block_hash| self.0.flat_storage_manager.chunk_view(shard_uid, block_hash))
+            .flatten();


nit: use .and_then instead of .map(..).flatten()

pugachAG · 2023-05-25T15:50:49Z

core/store/src/trie/state_parts.rs

+        };
+        let key_end = key_end.as_deref();
+
+        Ok(flat_storage_chunk_view.iter_flat_state_entries(key_begin, key_end))


nit: use iter_flat_state_entries(key_begin.as_deref(), key_end.as_deref()) instead of redefining variables, that looks a bit weird

core/store/src/trie/state_parts.rs

pugachAG · 2023-05-25T16:09:13Z

core/store/src/trie/state_parts.rs

+        let in_memory_created_nodes =
+            trie_values.iter().filter(|entry| !disk_read_hashes.contains(&hash(*entry))).count();
+        let trie_creation_duration = trie_creation_start.elapsed();
+        tracing::info!(


I suggest to consider avoiding manually formatting string for logging in general and use structured logging instead: %values_ref, ..., ?trie_creation_duration, .... This s a good practice because it still results in very readable logs, but avoids unnecessary string formatting overhead when log level is not enabled. Also I find that to be a bit easier to grep in the logs. Not a strong opinion here, just something to consider.

pugachAG · 2023-05-25T16:14:09Z

core/store/src/trie/state_parts.rs

+    ) -> Result<impl Iterator<Item = (Vec<u8>, Box<[u8]>)> + 'a, StorageError> {
+        let flat_storage_chunk_view = match &self.flat_storage_chunk_view {
+            None => {
+                return Err(StorageError::StorageInconsistentState(format!(


I guess we don't need format! here

Gennerate inner part of state part using flat storage using idea present in #8984. In short, if flat storage head corresponds to the state root for which we sync state, it is enough to read only boundary nodes, and inner trie part can be reconstructed using range of KV pairs from state. The main logic for that is contained in `Trie::get_trie_nodes_for_part_with_flat_storage`. It requires couple of minor changes: * now we allow creating "view" `Trie`s with flat storage as well. As before, we want to avoid creating non-view `Tries` because `TrieCache` accesses may be blocking for chunk processing * `get_head_hash` and `shard_uid` methods for `FlatStorage` allowing to make correct range query to flat storage * `FlatStateValue` moved to `primitives` to allow more general access * prometheus metrics * integration test checking that flat storage is used during normal block processing on client (or wait for #9090) https://nayduck.near.org/#/run/3023 Big sanity test `get_trie_nodes_for_part_with_flat_storage` covering all scenarios I could think of: * results with/without flat storage must match * result with incorrect flat storage must be an error * result with flat storage and missing intermediate node should be still okay

Gennerate inner part of state part using flat storage using idea present in near#8984. In short, if flat storage head corresponds to the state root for which we sync state, it is enough to read only boundary nodes, and inner trie part can be reconstructed using range of KV pairs from state. The main logic for that is contained in `Trie::get_trie_nodes_for_part_with_flat_storage`. It requires couple of minor changes: * now we allow creating "view" `Trie`s with flat storage as well. As before, we want to avoid creating non-view `Tries` because `TrieCache` accesses may be blocking for chunk processing * `get_head_hash` and `shard_uid` methods for `FlatStorage` allowing to make correct range query to flat storage * `FlatStateValue` moved to `primitives` to allow more general access * prometheus metrics * integration test checking that flat storage is used during normal block processing on client (or wait for near#9090) https://nayduck.near.org/#/run/3023 Big sanity test `get_trie_nodes_for_part_with_flat_storage` covering all scenarios I could think of: * results with/without flat storage must match * result with incorrect flat storage must be an error * result with flat storage and missing intermediate node should be still okay

Gennerate inner part of state part using flat storage using idea present in #8984. In short, if flat storage head corresponds to the state root for which we sync state, it is enough to read only boundary nodes, and inner trie part can be reconstructed using range of KV pairs from state. The main logic for that is contained in `Trie::get_trie_nodes_for_part_with_flat_storage`. It requires couple of minor changes: * now we allow creating "view" `Trie`s with flat storage as well. As before, we want to avoid creating non-view `Tries` because `TrieCache` accesses may be blocking for chunk processing * `get_head_hash` and `shard_uid` methods for `FlatStorage` allowing to make correct range query to flat storage * `FlatStateValue` moved to `primitives` to allow more general access * prometheus metrics * integration test checking that flat storage is used during normal block processing on client (or wait for #9090) https://nayduck.near.org/#/run/3023 Big sanity test `get_trie_nodes_for_part_with_flat_storage` covering all scenarios I could think of: * results with/without flat storage must match * result with incorrect flat storage must be an error * result with flat storage and missing intermediate node should be still okay

Longarithm force-pushed the fs-dump branch from 63d9baa to 00e9bad Compare April 19, 2023 13:40

Longarithm mentioned this pull request Apr 28, 2023

Initiative: Integrate Flat storage with State sync #8899

Closed

4 tasks

Looogarithm added 18 commits May 4, 2023 14:44

require kv pairs

7378a87

Merge branch 'master' into key-in-parts

222bc47

enum

d3023eb

fix usages

344a404

fix more usages

3d5a661

warn fixes

7f78011

comment

e32573a

Merge branch 'master' into key-in-parts

7f25c62

more state part tests

362f011

Merge branch 'master' into key-in-parts

5b67a6b

remove println

cba8cd2

trie values

b885848

docs

d81b97a

comment

c38e8bf

sanity

d5058d9

Merge branch 'master' into key-in-parts

9d55935

minor fix

be6ad78

Merge branch 'master' into key-in-parts

2001001

Longarithm force-pushed the fs-dump branch from b818e46 to e16c6a2 Compare May 12, 2023 20:50

cleanup

25b2727

Longarithm force-pushed the fs-dump branch from a3b2ca7 to 25b2727 Compare May 15, 2023 18:41

Merge branch 'master' into fs-dump

1014b78

This was referenced May 17, 2023

refactor: put sync_prev_prev_hash to obtain_state_part #9076

Merged

refactor: convenient iteration over flat state entries #9079

Merged

hide logic in obtain

bd74069

Longarithm force-pushed the fs-dump branch from 0722773 to bd74069 Compare May 19, 2023 14:55

Merge branch 'master' into fs-dump

1b21af9

Longarithm force-pushed the fs-dump branch from 897101a to e9267bc Compare May 22, 2023 12:23

Merge branch 'master' into fs-dump

d6872c9

Longarithm self-assigned this May 23, 2023

Longarithm added the A-storage Area: storage and databases label May 23, 2023

Longarithm linked an issue May 23, 2023 that may be closed by this pull request

Construct state sync parts using flat storage #8898

Closed

remove debug, polish impl

e64d336

Longarithm force-pushed the fs-dump branch from d45c779 to e64d336 Compare May 23, 2023 17:11

minor fix

7b03ff3

Longarithm requested review from nikurt, pugachAG and jbajic May 23, 2023 17:15

Longarithm marked this pull request as ready for review May 23, 2023 17:15

Longarithm requested a review from a team as a code owner May 23, 2023 17:15

Merge branch 'master' into fs-dump

45f6032

Longarithm changed the title ~~draft: state sync from flat storage~~ draft: construct state sync parts using flat storage May 23, 2023

Longarithm changed the title ~~draft: construct state sync parts using flat storage~~ feat: construct state sync parts using flat storage May 23, 2023

minor fix

684aa35

robin-near mentioned this pull request May 24, 2023

[Resharding] Offline prototype for using Flat Storage to reconstruct trie #9105

Closed

nikurt approved these changes May 25, 2023

View reviewed changes

pugachAG approved these changes May 25, 2023

View reviewed changes

apply suggestions

e4342b4

Longarithm added the S-automerge label May 25, 2023

Looogarithm added 3 commits May 25, 2023 23:46

Merge branch 'master' into fs-dump

07320ee

warnings

3e4fafa

warnings

160a8bc

near-bulldozer bot merged commit ff35a34 into near:master May 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: construct state sync parts using flat storage #8927

feat: construct state sync parts using flat storage #8927

Longarithm commented Apr 18, 2023 •

edited

Loading

pugachAG left a comment

pugachAG May 25, 2023

pugachAG May 25, 2023

pugachAG May 25, 2023

pugachAG May 25, 2023

pugachAG May 25, 2023

pugachAG May 25, 2023

feat: construct state sync parts using flat storage #8927

feat: construct state sync parts using flat storage #8927

Conversation

Longarithm commented Apr 18, 2023 • edited Loading

TODO

Testing

pugachAG left a comment

Choose a reason for hiding this comment

pugachAG May 25, 2023

Choose a reason for hiding this comment

pugachAG May 25, 2023

Choose a reason for hiding this comment

pugachAG May 25, 2023

Choose a reason for hiding this comment

pugachAG May 25, 2023

Choose a reason for hiding this comment

pugachAG May 25, 2023

Choose a reason for hiding this comment

pugachAG May 25, 2023

Choose a reason for hiding this comment

Longarithm commented Apr 18, 2023 •

edited

Loading