This repository has been archived by the owner on Jan 13, 2025. It is now read-only.
Make startup aware of Incremental Snapshots #19600
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
Codecov Report
@@ Coverage Diff @@
## master #19600 +/- ##
=========================================
- Coverage 82.5% 82.5% -0.1%
=========================================
Files 471 471
Lines 132112 132199 +87
=========================================
+ Hits 109096 109163 +67
- Misses 23016 23036 +20 |
brooksprumo
commented
Sep 3, 2021
ledger/src/blockstore_processor.rs
Outdated
Comment on lines
1189
to
1212
| if let Some(snapshot_config) = snapshot_config { | ||
| let block_height = new_root_bank.block_height(); | ||
| if block_height % snapshot_config.full_snapshot_archive_interval_slots == 0 { | ||
| snapshot_utils::snapshot_bank( | ||
| new_root_bank, | ||
| new_root_bank.src.slot_deltas(&new_root_bank.src.roots()), | ||
| &accounts_package_sender, | ||
| &snapshot_config.bank_snapshots_dir, | ||
| &snapshot_config.snapshot_archives_dir, | ||
| snapshot_config.snapshot_version, | ||
| snapshot_config.archive_format, | ||
| None, | ||
| Some(SnapshotType::FullSnapshot), | ||
| ) | ||
| .expect("Failed to snapshot bank while loading frozen banks"); | ||
| trace!( | ||
| "took bank snapshot for new root bank, block height: {}, slot: {}", | ||
| block_height, | ||
| *root | ||
| ); | ||
| *last_full_snapshot_slot = Some(*root); | ||
| } | ||
| } |
There was a problem hiding this comment.
This is where snapshots are taken while processing blocks that are roots, and also sets the new value for last_full_snapshot_slot.
There was a problem hiding this comment.
TODO: call clean before snapshot_bank
Update: DONE
| @@ -712,7 +712,7 @@ fn load_bank_forks( | |||
| let snapshot_archives_dir = | |||
| snapshot_archive_path.unwrap_or_else(|| blockstore.ledger_path().to_path_buf()); | |||
| Some(SnapshotConfig { | |||
| full_snapshot_archive_interval_slots: 0, // Value doesn't matter | |||
| full_snapshot_archive_interval_slots: Slot::MAX, | |||
There was a problem hiding this comment.
Cannot be zero because this value is used when processing the blockstore and is the modulo of the new roots. A value of 0 here is a divide-by-zero error.
Comment on lines
+138
to
+139
| accounts_package_channel: (AccountsPackageSender, AccountsPackageReceiver), | ||
| last_full_snapshot_slot: Option<Slot>, |
There was a problem hiding this comment.
The TVU now takes in an AccountsPackageSender/Receiver, and the last full snapshot slot. This comes from bank_forks::load() and blockstore_processor::do_process_blockstore_from_root().
brooksprumo
force-pushed
the
snapshot-startup-2
branch
from
acc370b to
e9cde56
Compare
carllin
approved these changes
Sep 7, 2021
brooksprumo
added
the
automerge
frits-metalogix
added a commit
to identity-com/solana
that referenced
this pull request
Nov 24, 2021
This reverts commit de70d6c.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Now that the background services are aware of incremental snapshots,
they need (1) the correct last full snapshot slot in order to clean
accounts correctly, and (2) all expected full snapshots to be available
when created an incremental snapshot based on them.
This commit fixes startup so both requirements from above are met.
At startup, the blockstore processor loads frozen banks. Some of these
banks may be roots, and some of these roots may cross the full snapshot
interval. If/when that happens, take a bank snapshot and queue the full
snapshot in the AccountsPackageSender. And at the end of startup,
return the last full snapshot slot to pass into the background services.
Fixes #19297
Overview
High level overview for
blockstore_processor::do_process_blockstore_from_root(). The new arglast_full_snapshot_slotis populated if we loaded from a full snapshot (vs starting from genesis). Thenload_frozen_forks()processes the blocks, and updates the value (last_full_snapshot_slot) in case a new root is found that crosses the full snapshot interval. Finally, the value is returned and gets passed along to AccountsBackgroundService for its starting value oflast_full_snapshot_slot.Additionally, if a new root is found during
load_frozen_forks()that crosses the full snapshot interval, now take a snapshot (withsnapshot_utils::snapshot_bank()).