[stateless_validation] Load memtrie async on catchup #10983

staffik · 2024-04-08T12:26:16Z

Context
Issue: #10982
Follow up to: #10820.

Modifies StateSync state machine so that memtrie load happens asynchronously on catchup.

Summary

Split chain.set_state_finalize() into:
- create_flat_storage_for_shard()
- schedule_load_memtrie()
- actual set_state_finalize()
^ we need it because creating flat storage and state finalize requires chain which cannot be passed in a message to the separate thread.
Code to trigger memtrie load in a separate thread, analogously to how apply state parts is done.
Modify shard sync stages:
- StateDownloadScheduling --> StateApplyScheduling
  - Just changed the name as it was confusing. What happens there is scheduling applying of state parts.
- StateDownloadApplying --> StateApplyComplete
  - What it actually did before was initializing flat storage and finalizing state update after state apply from previous stage.
  - Now it only initializes flat storage and schedules memtrie loading.
- StateDownloadComplete --> StateApplyFinalizing
  - Before it was just deciding what next stage to transit into.
  - Now it also contains the finalizing state update logic that was previously in the previous stage.

Integration tests are to be done as a part of: #10844.

codecov · 2024-04-09T11:45:31Z

Codecov Report

Attention: Patch coverage is 5.32544% with 160 lines in your changes are missing coverage. Please review.

Project coverage is 71.24%. Comparing base (4c0aa98) to head (b94876d).
Report is 20 commits behind head on master.

Files	Patch %	Lines
chain/client/src/sync/state.rs	5.81%	81 Missing ⚠️
chain/chain/src/chain.rs	0.00%	52 Missing ⚠️
chain/client/src/sync_jobs_actions.rs	0.00%	12 Missing ⚠️
chain/client-primitives/src/types.rs	0.00%	6 Missing ⚠️
chain/client/src/client_actions.rs	25.00%	3 Missing ⚠️
...hain/client/src/test_utils/sync_jobs_test_utils.rs	0.00%	3 Missing ⚠️
chain/client/src/sync_jobs_actor.rs	0.00%	1 Missing ⚠️
...client/src/test_utils/client_actions_test_utils.rs	0.00%	1 Missing ⚠️
tools/state-viewer/src/state_parts.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10983      +/-   ##
==========================================
- Coverage   71.33%   71.24%   -0.10%     
==========================================
  Files         760      758       -2     
  Lines      152288   152687     +399     
  Branches   152288   152687     +399     
==========================================
+ Hits       108640   108776     +136     
- Misses      39176    39450     +274     
+ Partials     4472     4461      -11

Flag	Coverage Δ
backward-compatibility	`0.24% <0.00%> (-0.01%)`	⬇️
db-migration	`0.24% <0.00%> (-0.01%)`	⬇️
genesis-check	`1.43% <0.00%> (-0.01%)`	⬇️
integration-tests	`36.90% <4.73%> (-0.08%)`	⬇️
linux	`69.69% <5.32%> (-0.13%)`	⬇️
linux-nightly	`70.74% <5.32%> (-0.07%)`	⬇️
macos	`54.39% <5.32%> (+0.10%)`	⬆️
pytests	`1.66% <0.00%> (-0.01%)`	⬇️
sanity-checks	`1.44% <0.00%> (-0.01%)`	⬇️
unittests	`66.92% <5.32%> (-0.06%)`	⬇️
upgradability	`0.29% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

chain/chain/src/chain.rs

chain/client/src/sync/state.rs

chain/client/src/client_actions.rs

tayfunelmas · 2024-04-09T21:57:59Z

A high level question: Do the state sync/catchup operations run on a separate thread anyways? since the loading of memtrie is part of sync/catchup, what is the added benefit of doing this loading in async way?

staffik · 2024-04-10T12:51:49Z

A high level question: Do the state sync/catchup operations run on a separate thread anyways? since the loading of memtrie is part of sync/catchup, what is the added benefit of doing this loading in async way?

State sync/catchup high level functions run on the client thread.
This is the main function that is called periodically to see if there is something to sync.
sync_shards_status() works as a state machine and some parts of sync are requested to be done on a separate thread, e.g. via chain::schedule_apply_state_parts().

shreyan-gupta

Looks good overall. Just confirming, this is only for the state sync with shard reshuffling and not for node startup right?

chain/client/src/sync/state.rs

shreyan-gupta · 2024-04-10T17:35:17Z

chain/client/src/sync/state.rs

+        // (these are set via callback from ClientActor - both for sync and catchup).
+        let mut shard_sync_done = false;
+        if let Some(result) = self.load_memtrie_results.remove(&shard_id) {
+            result


nit: Why the and_then and or_else here? It's slightly hard to read. Can't we do a match instead?

We want the or_else arm both if result is error or set_state_finalize() returns error. If I simply do match, I would have to duplicate the or_else arm code. I did not find a cleaner approach.

chain/client/src/sync_jobs_actions.rs

staffik · 2024-04-12T08:49:43Z

Looks good overall. Just confirming, this is only for the state sync with shard reshuffling and not for node startup right?

Yes, just for catchup before next epoch, not for startup.

Fix a wrong name for a state sync state I introduced in #10983. I called it `StateApplyComplete` while in fact the state apply is in progress.

staffik added the A-stateless-validation Area: stateless validation label Apr 8, 2024

staffik added 3 commits April 9, 2024 13:16

in progress

9fa9681

fix compile errors

7361b1e

move flat storage creation to a separate function

49f7830

staffik force-pushed the memtrie-load-async branch from bfede3c to 49f7830 Compare April 9, 2024 11:21

refactor

9b7c2c1

staffik marked this pull request as ready for review April 9, 2024 13:31

staffik requested a review from a team as a code owner April 9, 2024 13:31

staffik requested review from Longarithm, robin-near and tayfunelmas April 9, 2024 13:31

comment

2039d54

shreyan-gupta self-requested a review April 9, 2024 16:42

tayfunelmas reviewed Apr 9, 2024

View reviewed changes

chain/chain/src/chain.rs Show resolved Hide resolved

tayfunelmas reviewed Apr 9, 2024

View reviewed changes

chain/chain/src/chain.rs Show resolved Hide resolved

tayfunelmas reviewed Apr 9, 2024

View reviewed changes

chain/client/src/sync/state.rs Show resolved Hide resolved

tayfunelmas reviewed Apr 9, 2024

View reviewed changes

chain/client/src/client_actions.rs Outdated Show resolved Hide resolved

tayfunelmas approved these changes Apr 9, 2024

View reviewed changes

comments

9187cf2

shreyan-gupta approved these changes Apr 10, 2024

View reviewed changes

staffik added 2 commits April 11, 2024 17:25

Use shard_uid whenever possible

08beb0f

comment

b94876d

staffik added this pull request to the merge queue Apr 15, 2024

Merged via the queue into master with commit 579c53c Apr 15, 2024
26 of 29 checks passed

staffik deleted the memtrie-load-async branch April 15, 2024 13:09

staffik mentioned this pull request Jun 26, 2024

Fix naming for state apply state #11669

Merged

github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024

Fix naming for state apply state (#11669)

ea07739

Fix a wrong name for a state sync state I introduced in #10983. I called it `StateApplyComplete` while in fact the state apply is in progress.

staffik added a commit that referenced this pull request Jun 27, 2024

Fix naming for state apply state (#11669)

6a9cc4f

Fix a wrong name for a state sync state I introduced in #10983. I called it `StateApplyComplete` while in fact the state apply is in progress.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[stateless_validation] Load memtrie async on catchup #10983

[stateless_validation] Load memtrie async on catchup #10983

staffik commented Apr 8, 2024 •

edited

Loading

codecov bot commented Apr 9, 2024 •

edited

Loading

tayfunelmas commented Apr 9, 2024

staffik commented Apr 10, 2024 •

edited

Loading

shreyan-gupta left a comment

shreyan-gupta Apr 10, 2024

staffik Apr 11, 2024

staffik commented Apr 12, 2024

[stateless_validation] Load memtrie async on catchup #10983

[stateless_validation] Load memtrie async on catchup #10983

Conversation

staffik commented Apr 8, 2024 • edited Loading

codecov bot commented Apr 9, 2024 • edited Loading

Codecov Report

tayfunelmas commented Apr 9, 2024

staffik commented Apr 10, 2024 • edited Loading

shreyan-gupta left a comment

Choose a reason for hiding this comment

shreyan-gupta Apr 10, 2024

Choose a reason for hiding this comment

staffik Apr 11, 2024

Choose a reason for hiding this comment

staffik commented Apr 12, 2024

staffik commented Apr 8, 2024 •

edited

Loading

codecov bot commented Apr 9, 2024 •

edited

Loading

staffik commented Apr 10, 2024 •

edited

Loading