Skip to content

Commit

Permalink
Do not purge old snapshot archives inside of archive_snapshot_package…
Browse files Browse the repository at this point in the history 
…() (solana-labs#1417)
  • Loading branch information
@brooksprumo
brooksprumo authored May 21, 2024
1 parent b711bda commit 8ea3a17
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 87 deletions.
72 changes: 40 additions & 32 deletions core/src/snapshot_packager_service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use {
crossbeam_channel::{Receiver, Sender},
snapshot_gossip_manager::SnapshotGossipManager,
solana_gossip::cluster_info::ClusterInfo,
solana_measure::measure_us,
solana_measure::{measure::Measure, measure_us},
solana_perf::thread::renice_this_thread,
solana_runtime::{
snapshot_archive_info::SnapshotArchiveInfoGetter,
Expand Down Expand Up @@ -67,41 +67,44 @@ impl SnapshotPackagerService {
info!("handling snapshot package: {snapshot_package:?}");
let enqueued_time = snapshot_package.enqueued.elapsed();

let (purge_bank_snapshots_time_us, handling_time_us) = measure_us!({
// Archiving the snapshot package is not allowed to fail.
// AccountsBackgroundService calls `clean_accounts()` with a value for
// last_full_snapshot_slot that requires this archive call to succeed.
let result = snapshot_utils::archive_snapshot_package(
&snapshot_package,
&snapshot_config.full_snapshot_archives_dir,
&snapshot_config.incremental_snapshot_archives_dir,
snapshot_config.maximum_full_snapshot_archives_to_retain,
snapshot_config.maximum_incremental_snapshot_archives_to_retain,
let measure_handling = Measure::start("");

// Archiving the snapshot package is not allowed to fail.
// AccountsBackgroundService calls `clean_accounts()` with a value for
// last_full_snapshot_slot that requires this archive call to succeed.
let result = snapshot_utils::archive_snapshot_package(
&snapshot_package,
);
if let Err(err) = result {
error!("Stopping SnapshotPackagerService! Fatal error while archiving snapshot package: {err}");
exit.store(true, Ordering::Relaxed);
break;
}

if let Some(snapshot_gossip_manager) = snapshot_gossip_manager.as_mut() {
snapshot_gossip_manager.push_snapshot_hash(
snapshot_package.snapshot_kind,
(snapshot_package.slot(), *snapshot_package.hash()),
);
if let Err(err) = result {
error!("Stopping SnapshotPackagerService! Fatal error while archiving snapshot package: {err}");
exit.store(true, Ordering::Relaxed);
break;
}
}

if let Some(snapshot_gossip_manager) = snapshot_gossip_manager.as_mut() {
snapshot_gossip_manager.push_snapshot_hash(
snapshot_package.snapshot_kind,
(snapshot_package.slot(), *snapshot_package.hash()),
);
}
let (_, purge_archives_time_us) = measure_us!(snapshot_utils::purge_old_snapshot_archives(
&snapshot_config.full_snapshot_archives_dir,
&snapshot_config.incremental_snapshot_archives_dir,
snapshot_config.maximum_full_snapshot_archives_to_retain,
snapshot_config.maximum_incremental_snapshot_archives_to_retain,
));

// Now that this snapshot package has been archived, it is safe to remove
// all bank snapshots older than this slot. We want to keep the bank
// snapshot *at this slot* so that it can be used during restarts, when
// booting from local state.
measure_us!(snapshot_utils::purge_bank_snapshots_older_than_slot(
&snapshot_config.bank_snapshots_dir,
snapshot_package.slot(),
))
.1
});
// Now that this snapshot package has been archived, it is safe to remove
// all bank snapshots older than this slot. We want to keep the bank
// snapshot *at this slot* so that it can be used during restarts, when
// booting from local state.
let (_, purge_bank_snapshots_time_us) = measure_us!(snapshot_utils::purge_bank_snapshots_older_than_slot(
&snapshot_config.bank_snapshots_dir,
snapshot_package.slot(),
));

let handling_time_us = measure_handling.end_as_us();
datapoint_info!(
"snapshot_packager_service",
(
Expand All @@ -121,6 +124,11 @@ impl SnapshotPackagerService {
purge_bank_snapshots_time_us,
i64
),
(
"purge_old_archives_time_us",
purge_archives_time_us,
i64
),
);
}
info!("SnapshotPackagerService has stopped");
Expand Down
13 changes: 1 addition & 12 deletions core/tests/snapshots.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,14 +275,7 @@ fn run_bank_forks_snapshot_n<F>(
None,
);
let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash.into());
snapshot_utils::archive_snapshot_package(
&snapshot_package,
&snapshot_config.full_snapshot_archives_dir,
&snapshot_config.incremental_snapshot_archives_dir,
snapshot_config.maximum_full_snapshot_archives_to_retain,
snapshot_config.maximum_incremental_snapshot_archives_to_retain,
)
.unwrap();
snapshot_utils::archive_snapshot_package(&snapshot_package).unwrap();

// Restore bank from snapshot
let (_tmp_dir, temporary_accounts_dir) = create_tmp_accounts_dir_for_tests();
Expand Down Expand Up @@ -865,8 +858,6 @@ fn make_full_snapshot_archive(
bank.get_snapshot_storages(None),
snapshot_config.archive_format,
snapshot_config.snapshot_version,
snapshot_config.maximum_full_snapshot_archives_to_retain,
snapshot_config.maximum_incremental_snapshot_archives_to_retain,
)?;

Ok(())
Expand Down Expand Up @@ -904,8 +895,6 @@ fn make_incremental_snapshot_archive(
storages,
snapshot_config.archive_format,
snapshot_config.snapshot_version,
snapshot_config.maximum_full_snapshot_archives_to_retain,
snapshot_config.maximum_incremental_snapshot_archives_to_retain,
)?;

Ok(())
Expand Down
31 changes: 2 additions & 29 deletions runtime/src/snapshot_bank_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ use {
collections::{HashMap, HashSet},
fs,
io::{BufWriter, Write},
num::NonZeroUsize,
ops::RangeInclusive,
path::{Path, PathBuf},
sync::{atomic::AtomicBool, Arc},
Expand Down Expand Up @@ -1056,10 +1055,6 @@ pub fn bank_to_full_snapshot_archive(
snapshot_storages,
archive_format,
snapshot_version,
// Since bank_to_snapshot_archive() is not called as part of normal validator operation,
// do *not* purge any snapshot archives; leave that up to the node operator.
NonZeroUsize::MAX,
NonZeroUsize::MAX,
)
}

Expand Down Expand Up @@ -1109,10 +1104,6 @@ pub fn bank_to_incremental_snapshot_archive(
snapshot_storages,
archive_format,
snapshot_version,
// Since bank_to_snapshot_archive() is not called as part of normal validator operation,
// do *not* purge any snapshot archives; leave that up to the node operator.
NonZeroUsize::MAX,
NonZeroUsize::MAX,
)
}

Expand All @@ -1127,8 +1118,6 @@ fn package_and_archive_full_snapshot(
snapshot_storages: Vec<Arc<AccountStorageEntry>>,
archive_format: ArchiveFormat,
snapshot_version: SnapshotVersion,
maximum_full_snapshot_archives_to_retain: NonZeroUsize,
maximum_incremental_snapshot_archives_to_retain: NonZeroUsize,
) -> snapshot_utils::Result<FullSnapshotArchiveInfo> {
let accounts_package = AccountsPackage::new_for_snapshot(
AccountsPackageKind::Snapshot(SnapshotKind::FullSnapshot),
Expand All @@ -1153,13 +1142,7 @@ fn package_and_archive_full_snapshot(
);

let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash.into());
archive_snapshot_package(
&snapshot_package,
full_snapshot_archives_dir,
incremental_snapshot_archives_dir,
maximum_full_snapshot_archives_to_retain,
maximum_incremental_snapshot_archives_to_retain,
)?;
archive_snapshot_package(&snapshot_package)?;

Ok(FullSnapshotArchiveInfo::new(
snapshot_package.snapshot_archive_info,
Expand All @@ -1178,8 +1161,6 @@ fn package_and_archive_incremental_snapshot(
snapshot_storages: Vec<Arc<AccountStorageEntry>>,
archive_format: ArchiveFormat,
snapshot_version: SnapshotVersion,
maximum_full_snapshot_archives_to_retain: NonZeroUsize,
maximum_incremental_snapshot_archives_to_retain: NonZeroUsize,
) -> snapshot_utils::Result<IncrementalSnapshotArchiveInfo> {
let accounts_package = AccountsPackage::new_for_snapshot(
AccountsPackageKind::Snapshot(SnapshotKind::IncrementalSnapshot(
Expand Down Expand Up @@ -1222,13 +1203,7 @@ fn package_and_archive_incremental_snapshot(
);

let snapshot_package = SnapshotPackage::new(accounts_package, incremental_accounts_hash.into());
archive_snapshot_package(
&snapshot_package,
full_snapshot_archives_dir,
incremental_snapshot_archives_dir,
maximum_full_snapshot_archives_to_retain,
maximum_incremental_snapshot_archives_to_retain,
)?;
archive_snapshot_package(&snapshot_package)?;

Ok(IncrementalSnapshotArchiveInfo::new(
incremental_snapshot_base_slot,
Expand Down Expand Up @@ -2773,8 +2748,6 @@ mod tests {
snapshot_storages,
snapshot_config.archive_format,
snapshot_config.snapshot_version,
snapshot_config.maximum_full_snapshot_archives_to_retain,
snapshot_config.maximum_incremental_snapshot_archives_to_retain,
)
.unwrap();
}
Expand Down
15 changes: 1 addition & 14 deletions runtime/src/snapshot_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -730,13 +730,7 @@ pub fn remove_tmp_snapshot_archives(snapshot_archives_dir: impl AsRef<Path>) {
}

/// Make a snapshot archive out of the snapshot package
pub fn archive_snapshot_package(
snapshot_package: &SnapshotPackage,
full_snapshot_archives_dir: impl AsRef<Path>,
incremental_snapshot_archives_dir: impl AsRef<Path>,
maximum_full_snapshot_archives_to_retain: NonZeroUsize,
maximum_incremental_snapshot_archives_to_retain: NonZeroUsize,
) -> Result<()> {
pub fn archive_snapshot_package(snapshot_package: &SnapshotPackage) -> Result<()> {
use ArchiveSnapshotPackageError as E;
const SNAPSHOTS_DIR: &str = "snapshots";
const ACCOUNTS_DIR: &str = "accounts";
Expand Down Expand Up @@ -884,13 +878,6 @@ pub fn archive_snapshot_package(
fs::rename(&archive_path, snapshot_package.path())
.map_err(|err| E::MoveArchive(err, archive_path, snapshot_package.path().clone()))?;

purge_old_snapshot_archives(
full_snapshot_archives_dir,
incremental_snapshot_archives_dir,
maximum_full_snapshot_archives_to_retain,
maximum_incremental_snapshot_archives_to_retain,
);

timer.stop();
info!(
"Successfully created {}. slot: {}, elapsed ms: {}, size: {}",
Expand Down

0 comments on commit 8ea3a17

Please sign in to comment.