[REVERTED] Make startup aware of Incremental Snapshots #19550
Conversation
| 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().
| timings: BankFromArchiveTimings, | ||
| mut last_full_snapshot_slot: Option<Slot>, |
There was a problem hiding this comment.
High level overview for do_process_blockstore_from_root(). The new arg last_full_snapshot_slot is populated if we loaded from a full snapshot (vs starting from genesis). Then load_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 of last_full_snapshot_slot.
| 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.
brooksprumo
force-pushed
the
snapshot-startup
branch
from
ba8aeb8 to
e68e390
Compare
Codecov Report
@@ Coverage Diff @@
## master #19550 +/- ##
========================================
Coverage 82.5% 82.5%
========================================
Files 468 468
Lines 131542 131662 +120
========================================
+ Hits 108540 108693 +153
+ Misses 23002 22969 -33 |
Now that the background services are aware of incremental snapshots, they need (1) the corect 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 solana-labs#19297
brooksprumo
force-pushed
the
snapshot-startup
branch
from
e68e390 to
fede8d6
Compare
| @@ -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.
This reverts commit d45ced0.
|
Ugh, I was on the wrong PR and merged this one accidentally :( It has been reverted. New PR is #19600 |
This reverts commit d45ced0.
brooksprumo
changed the title
…" (solana-labs#19599) This reverts commit d45ced0.
…abs#19550)" (solana-labs#19599)" This reverts commit 5e87fc8.
This reverts commit 15235f4.
Now that the background services are aware of incremental snapshots,
they need (1) the corect 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