feat: finish code for background flat storage creation #8053
Conversation
Longarithm
changed the title
There was a problem hiding this comment.
Looks solid to me, great to see this progress!
@mzhangmzz I think you should also take a look before we merge, feels like your expertise around the chain and flat state are required here.
@Longarithm Do you plan to add tests to this PR or will it be a follow-up? I would tend towards a separate follow-up PR but it's up to you.
Co-authored-by: Jakob Meier <mail@jakobmeier.ch>
Yeah, they will be in follow-up PRs. |
There was a problem hiding this comment.
The PR looks great! Thanks for the awesome work and sorry for the super late review.
Most of my comments are about adding more comments :) Let's try to make the code easier to read for other people and the future us. I approved it to unblock, but please address the comments.
| use near_store::Store; | ||
| use near_store::{Trie, TrieDBStorage, TrieTraversalItem}; | ||
| use std::sync::atomic::AtomicU64; | ||
| use std::sync::Arc; | ||
| use tracing::debug; | ||
| use tracing::info; | ||
|
|
There was a problem hiding this comment.
Maybe add a paragraph at the beginning of this file to describe on a high level how flat state is migrated, what the steps are, and how FlatStateCreator is used in the code. I know that you already have comments throughout the code, but I think it is still valuable to have an overview to link everything together and help the readers to have a general picture before they dive into the code.
| merged_delta.merge(delta.as_ref()); | ||
| } | ||
|
|
||
| if old_flat_head != flat_head { |
There was a problem hiding this comment.
is it possible that old_flat_head == flat_head == final_head?
There was a problem hiding this comment.
In reality - no, because final head should move forward at least once during FetchingState step.
And even if it is the case, it is fine to wait on this step until final head moves forward.
| // If we reached chain final head, we can finish catchup and finally create flat storage. | ||
| store_helper::finish_catchup(&mut store_update, shard_id); | ||
| store_update.commit()?; | ||
| debug!(target: "chain", %shard_id, %flat_head, %height, "Creating flat storage"); |
There was a problem hiding this comment.
Is this debug statement intended? Should it print something like flat storage done?
There was a problem hiding this comment.
Yeah, it is intended.
I agree that "Flat storage creation done" sounds better. Also changed debug! to info! to places where major part of work is finished so users will be aware of status but there won't be much spam in logs.
| let mut store_update = chain_store.store().store_update(); | ||
| store_helper::set_flat_head(&mut store_update, shard_id, &block_hash); | ||
| store_helper::set_fetching_state_step(&mut store_update, shard_id, 0u64); | ||
| store_helper::set_fetching_state_status(&mut store_update, shard_id, status); |
There was a problem hiding this comment.
Should we store flat head here with a different prefix, maybe something like set_temporary_flat_head to distinguish it from when flat head is actually set and the flat state is ready to use? This way, there is no way the code can accidentally create a flat storage if flat state is not ready.
There was a problem hiding this comment.
We probably should. Let's do it separately because this change is already quite big and write some unit test checking that FS can't be created accidentally.
Co-authored-by: Jakob Meier <mail@jakobmeier.ch>
Implement two remaining steps for background flat storage creation:
Fetching state
We split the state into several parts and fetch them one by one. Number of parts is based on the state size limit, which I set to 10 MiB. Then, fetching is executed in several steps so that we could save intermediate results. Currently each step includes 20 state parts. They are fetched using threads of a separate rayon pool, which size is limited by 4 threads so that it doesn't affect block processing.
Here I also introduce
FetchingStateStatuswhich defines the current progress. It makes testing more convenient: for lightweight tests it is enough to fetch only one part, alhough on production we need thousands of parts.After state for a shard is fully fetched, we start catching up - which means that we move flat storage head forward and apply all saved deltas, limiting it by 50 blocks at once.
After we fully caught up, we finally create flat storage state.
Testing
test_flat_storage_creation.