From 9d9482b9d8f9731610b5587b28580f00b726aa9b Mon Sep 17 00:00:00 2001 From: Brooks Prumo Date: Mon, 6 Sep 2021 18:01:56 -0500 Subject: [PATCH] Plumb `maximum_incremental_snapshot_archives_to_retain` (#19640) --- core/src/snapshot_packager_service.rs | 2 + core/src/validator.rs | 1 + core/tests/snapshots.rs | 9 ++-- download-utils/src/lib.rs | 7 +-- ledger-tool/src/main.rs | 7 ++- local-cluster/tests/local_cluster.rs | 1 + replica-node/src/replica_node.rs | 1 + runtime/src/snapshot_utils.rs | 64 ++++++++++++++++++--------- validator/src/main.rs | 9 ++-- 9 files changed, 69 insertions(+), 32 deletions(-) diff --git a/core/src/snapshot_packager_service.rs b/core/src/snapshot_packager_service.rs index 2d754f3d5..81483c9f4 100644 --- a/core/src/snapshot_packager_service.rs +++ b/core/src/snapshot_packager_service.rs @@ -54,6 +54,7 @@ impl SnapshotPackagerService { snapshot_utils::archive_snapshot_package( &snapshot_package, snapshot_config.maximum_full_snapshot_archives_to_retain, + snapshot_config.maximum_incremental_snapshot_archives_to_retain, ) .expect("failed to archive snapshot package"); @@ -190,6 +191,7 @@ mod tests { snapshot_utils::archive_snapshot_package( &snapshot_package, snapshot_utils::DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + snapshot_utils::DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, ) .unwrap(); diff --git a/core/src/validator.rs b/core/src/validator.rs index 52b03c552..2c4df6cc5 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -1238,6 +1238,7 @@ fn new_banks_from_ledger( &snapshot_config.snapshot_archives_dir, snapshot_config.archive_format, snapshot_config.maximum_full_snapshot_archives_to_retain, + snapshot_config.maximum_incremental_snapshot_archives_to_retain, ) .unwrap_or_else(|err| { error!("Unable to create snapshot: {}", err); diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index e96a8eed0..007204f8c 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -299,7 +299,8 @@ mod tests { let snapshot_package = SnapshotPackage::from(accounts_package); snapshot_utils::archive_snapshot_package( &snapshot_package, - DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + snapshot_config.maximum_full_snapshot_archives_to_retain, + snapshot_config.maximum_incremental_snapshot_archives_to_retain, ) .unwrap(); @@ -387,7 +388,7 @@ mod tests { let saved_slot = 4; let mut saved_archive_path = None; - for forks in 0..snapshot_utils::MAX_BANK_SNAPSHOTS + 2 { + for forks in 0..snapshot_utils::MAX_BANK_SNAPSHOTS_TO_RETAIN + 2 { let bank = Bank::new_from_parent( &bank_forks[forks as u64], &Pubkey::default(), @@ -483,7 +484,7 @@ mod tests { assert!(bank_snapshots .into_iter() .map(|path| path.slot) - .eq(3..=snapshot_utils::MAX_BANK_SNAPSHOTS as u64 + 2)); + .eq(3..=snapshot_utils::MAX_BANK_SNAPSHOTS_TO_RETAIN as u64 + 2)); // Create a SnapshotPackagerService to create tarballs from all the pending // SnapshotPackage's on the channel. By the time this service starts, we have already @@ -776,6 +777,7 @@ mod tests { 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(()) @@ -812,6 +814,7 @@ mod tests { 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(()) diff --git a/download-utils/src/lib.rs b/download-utils/src/lib.rs index 4f3b325d3..f813128e2 100644 --- a/download-utils/src/lib.rs +++ b/download-utils/src/lib.rs @@ -249,13 +249,14 @@ pub fn download_snapshot<'a, 'b>( snapshot_archives_dir: &Path, desired_snapshot_hash: (Slot, Hash), use_progress_bar: bool, - maximum_snapshots_to_retain: usize, + maximum_full_snapshot_archives_to_retain: usize, + maximum_incremental_snapshot_archives_to_retain: usize, progress_notify_callback: &'a mut DownloadProgressCallbackOption<'b>, ) -> Result<(), String> { snapshot_utils::purge_old_snapshot_archives( snapshot_archives_dir, - maximum_snapshots_to_retain, - snapshot_utils::DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, + maximum_full_snapshot_archives_to_retain, + maximum_incremental_snapshot_archives_to_retain, ); for compression in &[ diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index a5ed2ac8e..c0ef0be8d 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -2018,8 +2018,10 @@ fn main() { }) }); - let maximum_snapshots_to_retain = + let maximum_full_snapshot_archives_to_retain = value_t_or_exit!(arg_matches, "maximum_snapshots_to_retain", usize); + let maximum_incremental_snapshot_archives_to_retain = + snapshot_utils::DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN; let genesis_config = open_genesis_config_by(&ledger_path, arg_matches); let blockstore = open_blockstore( &ledger_path, @@ -2235,7 +2237,8 @@ fn main() { Some(snapshot_version), output_directory, ArchiveFormat::TarZstd, - maximum_snapshots_to_retain, + maximum_full_snapshot_archives_to_retain, + maximum_incremental_snapshot_archives_to_retain, ) .unwrap_or_else(|err| { eprintln!("Unable to create snapshot: {}", err); diff --git a/local-cluster/tests/local_cluster.rs b/local-cluster/tests/local_cluster.rs index e5272f97d..948116949 100644 --- a/local-cluster/tests/local_cluster.rs +++ b/local-cluster/tests/local_cluster.rs @@ -1708,6 +1708,7 @@ fn test_snapshot_download() { archive_snapshot_hash, false, snapshot_utils::DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + snapshot_utils::DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, &mut None, ) .unwrap(); diff --git a/replica-node/src/replica_node.rs b/replica-node/src/replica_node.rs index 871a7f464..2aecff8a7 100644 --- a/replica-node/src/replica_node.rs +++ b/replica-node/src/replica_node.rs @@ -95,6 +95,7 @@ fn initialize_from_snapshot( replica_config.snapshot_info, false, snapshot_config.maximum_full_snapshot_archives_to_retain, + snapshot_config.maximum_incremental_snapshot_archives_to_retain, &mut None, ) .unwrap(); diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 62c0234c8..8f593a412 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -45,12 +45,12 @@ use { pub const SNAPSHOT_STATUS_CACHE_FILE_NAME: &str = "status_cache"; -pub const MAX_BANK_SNAPSHOTS: usize = 8; // Save some snapshots but not too many const MAX_SNAPSHOT_DATA_FILE_SIZE: u64 = 32 * 1024 * 1024 * 1024; // 32 GiB const VERSION_STRING_V1_2_0: &str = "1.2.0"; const DEFAULT_SNAPSHOT_VERSION: SnapshotVersion = SnapshotVersion::V1_2_0; pub(crate) const TMP_BANK_SNAPSHOT_PREFIX: &str = "tmp-bank-snapshot-"; pub const TMP_SNAPSHOT_ARCHIVE_PREFIX: &str = "tmp-snapshot-archive-"; +pub const MAX_BANK_SNAPSHOTS_TO_RETAIN: usize = 8; // Save some bank snapshots but not too many pub const DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN: usize = 2; pub const DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN: usize = 4; pub const FULL_SNAPSHOT_ARCHIVE_FILENAME_REGEX: &str = r"^snapshot-(?P[[:digit:]]+)-(?P[[:alnum:]]+)\.(?Ptar|tar\.bz2|tar\.zst|tar\.gz)$"; @@ -242,7 +242,8 @@ pub fn remove_tmp_snapshot_archives(snapshot_archives_dir: impl AsRef) { /// Make a snapshot archive out of the snapshot package pub fn archive_snapshot_package( snapshot_package: &SnapshotPackage, - maximum_snapshot_archives_to_retain: usize, + maximum_full_snapshot_archives_to_retain: usize, + maximum_incremental_snapshot_archives_to_retain: usize, ) -> Result<()> { info!( "Generating snapshot archive for slot {}", @@ -374,8 +375,8 @@ pub fn archive_snapshot_package( purge_old_snapshot_archives( tar_dir, - maximum_snapshot_archives_to_retain, - DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, + maximum_full_snapshot_archives_to_retain, + maximum_incremental_snapshot_archives_to_retain, ); timer.stop(); @@ -1552,12 +1553,12 @@ where bank_snapshot_infos .into_iter() .rev() - .skip(MAX_BANK_SNAPSHOTS) + .skip(MAX_BANK_SNAPSHOTS_TO_RETAIN) .for_each(|bank_snapshot_info| { let r = remove_bank_snapshot(bank_snapshot_info.slot, &bank_snapshots_dir); if r.is_err() { warn!( - "Couldn't remove snapshot at: {}", + "Couldn't remove bank snapshot at: {}", bank_snapshot_info.snapshot_path.display() ); } @@ -1630,7 +1631,8 @@ pub fn bank_to_full_snapshot_archive( snapshot_version: Option, snapshot_archives_dir: impl AsRef, archive_format: ArchiveFormat, - maximum_snapshots_to_retain: usize, + maximum_full_snapshot_archives_to_retain: usize, + maximum_incremental_snapshot_archives_to_retain: usize, ) -> Result { let snapshot_version = snapshot_version.unwrap_or_default(); @@ -1654,7 +1656,8 @@ pub fn bank_to_full_snapshot_archive( snapshot_storages, archive_format, snapshot_version, - maximum_snapshots_to_retain, + maximum_full_snapshot_archives_to_retain, + maximum_incremental_snapshot_archives_to_retain, ) } @@ -1671,7 +1674,8 @@ pub fn bank_to_incremental_snapshot_archive( snapshot_version: Option, snapshot_archives_dir: impl AsRef, archive_format: ArchiveFormat, - maximum_snapshots_to_retain: usize, + maximum_full_snapshot_archives_to_retain: usize, + maximum_incremental_snapshot_archives_to_retain: usize, ) -> Result { let snapshot_version = snapshot_version.unwrap_or_default(); @@ -1697,7 +1701,8 @@ pub fn bank_to_incremental_snapshot_archive( snapshot_storages, archive_format, snapshot_version, - maximum_snapshots_to_retain, + maximum_full_snapshot_archives_to_retain, + maximum_incremental_snapshot_archives_to_retain, ) } @@ -1710,7 +1715,8 @@ pub fn package_and_archive_full_snapshot( snapshot_storages: SnapshotStorages, archive_format: ArchiveFormat, snapshot_version: SnapshotVersion, - maximum_snapshots_to_retain: usize, + maximum_full_snapshot_archives_to_retain: usize, + maximum_incremental_snapshot_archives_to_retain: usize, ) -> Result { let accounts_package = AccountsPackage::new( bank, @@ -1726,7 +1732,11 @@ pub fn package_and_archive_full_snapshot( )?; let snapshot_package = SnapshotPackage::from(accounts_package); - archive_snapshot_package(&snapshot_package, maximum_snapshots_to_retain)?; + archive_snapshot_package( + &snapshot_package, + maximum_full_snapshot_archives_to_retain, + maximum_incremental_snapshot_archives_to_retain, + )?; Ok(FullSnapshotArchiveInfo::new( snapshot_package.snapshot_archive_info, @@ -1744,7 +1754,8 @@ pub fn package_and_archive_incremental_snapshot( snapshot_storages: SnapshotStorages, archive_format: ArchiveFormat, snapshot_version: SnapshotVersion, - maximum_snapshots_to_retain: usize, + maximum_full_snapshot_archives_to_retain: usize, + maximum_incremental_snapshot_archives_to_retain: usize, ) -> Result { let accounts_package = AccountsPackage::new( bank, @@ -1762,7 +1773,11 @@ pub fn package_and_archive_incremental_snapshot( )?; let snapshot_package = SnapshotPackage::from(accounts_package); - archive_snapshot_package(&snapshot_package, maximum_snapshots_to_retain)?; + archive_snapshot_package( + &snapshot_package, + maximum_full_snapshot_archives_to_retain, + maximum_incremental_snapshot_archives_to_retain, + )?; Ok(IncrementalSnapshotArchiveInfo::new( incremental_snapshot_base_slot, @@ -2544,7 +2559,8 @@ mod tests { None, snapshot_archives_dir.path(), snapshot_archive_format, - 1, + DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, ) .unwrap(); @@ -2634,7 +2650,8 @@ mod tests { None, snapshot_archives_dir.path(), snapshot_archive_format, - std::usize::MAX, + DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, ) .unwrap(); @@ -2710,7 +2727,8 @@ mod tests { None, snapshot_archives_dir.path(), snapshot_archive_format, - std::usize::MAX, + DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, ) .unwrap(); @@ -2742,7 +2760,8 @@ mod tests { None, snapshot_archives_dir.path(), snapshot_archive_format, - std::usize::MAX, + DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, ) .unwrap(); @@ -2808,7 +2827,8 @@ mod tests { None, &snapshot_archives_dir, snapshot_archive_format, - std::usize::MAX, + DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, ) .unwrap(); @@ -2840,7 +2860,8 @@ mod tests { None, &snapshot_archives_dir, snapshot_archive_format, - std::usize::MAX, + DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, ) .unwrap(); @@ -2939,6 +2960,7 @@ mod tests { snapshot_archives_dir.path(), snapshot_archive_format, DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, ) .unwrap(); @@ -2980,6 +3002,7 @@ mod tests { snapshot_archives_dir.path(), snapshot_archive_format, DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, ) .unwrap(); let (deserialized_bank, _) = bank_from_snapshot_archives( @@ -3040,6 +3063,7 @@ mod tests { snapshot_archives_dir.path(), snapshot_archive_format, DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, + DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN, ) .unwrap(); diff --git a/validator/src/main.rs b/validator/src/main.rs index f83b11b11..dbeccc86d 100644 --- a/validator/src/main.rs +++ b/validator/src/main.rs @@ -925,19 +925,20 @@ fn rpc_bootstrap( gossip.take().unwrap(); cluster_info.save_contact_info(); gossip_exit_flag.store(true, Ordering::Relaxed); - let maximum_snapshots_to_retain = if let Some(snapshot_config) = + let (maximum_full_snapshot_archives_to_retain, maximum_incremental_snapshot_archives_to_retain) = if let Some(snapshot_config) = validator_config.snapshot_config.as_ref() { - snapshot_config.maximum_full_snapshot_archives_to_retain + (snapshot_config.maximum_full_snapshot_archives_to_retain, snapshot_config.maximum_incremental_snapshot_archives_to_retain) } else { - DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN + (DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN, DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN) }; let ret = download_snapshot( &rpc_contact_info.rpc, snapshot_archives_dir, snapshot_hash, use_progress_bar, - maximum_snapshots_to_retain, + maximum_full_snapshot_archives_to_retain, + maximum_incremental_snapshot_archives_to_retain, &mut Some(Box::new(|download_progress: &DownloadProgressRecord| { debug!("Download progress: {:?}", download_progress);