diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs index 26f3456f6f..f1365781c4 100644 --- a/core/src/accounts_hash_verifier.rs +++ b/core/src/accounts_hash_verifier.rs @@ -145,6 +145,10 @@ impl AccountsHashVerifier { assert_eq!(expected_hash, hash); }; measure_hash.stop(); + solana_runtime::serde_snapshot::reserialize_bank( + accounts_package.snapshot_links.path(), + accounts_package.slot, + ); datapoint_info!( "accounts_hash_verifier", ("calculate_hash", measure_hash.as_us(), i64), diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 32bddd146f..3a7e70a7d3 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -166,10 +166,10 @@ mod tests { old_genesis_config: &GenesisConfig, account_paths: &[PathBuf], ) { - let (snapshot_path, snapshot_archives_dir) = old_bank_forks + let snapshot_archives_dir = old_bank_forks .snapshot_config .as_ref() - .map(|c| (&c.bank_snapshots_dir, &c.snapshot_archives_dir)) + .map(|c| &c.snapshot_archives_dir) .unwrap(); let old_last_bank = old_bank_forks.get(old_last_slot).unwrap(); @@ -213,12 +213,6 @@ mod tests { .unwrap() .clone(); assert_eq!(*bank, deserialized_bank); - - let bank_snapshots = snapshot_utils::get_bank_snapshots(&snapshot_path); - - for p in bank_snapshots { - snapshot_utils::remove_bank_snapshot(p.slot, &snapshot_path).unwrap(); - } } // creates banks up to "last_slot" and runs the input function `f` on each bank created @@ -274,7 +268,7 @@ mod tests { let snapshot_config = &snapshot_test_config.snapshot_config; let bank_snapshots_dir = &snapshot_config.bank_snapshots_dir; let last_bank_snapshot_info = - snapshot_utils::get_highest_bank_snapshot_info(bank_snapshots_dir) + snapshot_utils::get_highest_bank_snapshot_pre(bank_snapshots_dir) .expect("no bank snapshots found in path"); let accounts_package = AccountsPackage::new( last_bank, @@ -289,6 +283,10 @@ mod tests { Some(SnapshotType::FullSnapshot), ) .unwrap(); + solana_runtime::serde_snapshot::reserialize_bank( + accounts_package.snapshot_links.path(), + accounts_package.slot, + ); let snapshot_package = SnapshotPackage::from(accounts_package); snapshot_utils::archive_snapshot_package( &snapshot_package, @@ -472,7 +470,7 @@ mod tests { // currently sitting in the channel snapshot_utils::purge_old_bank_snapshots(bank_snapshots_dir); - let mut bank_snapshots = snapshot_utils::get_bank_snapshots(&bank_snapshots_dir); + let mut bank_snapshots = snapshot_utils::get_bank_snapshots_pre(&bank_snapshots_dir); bank_snapshots.sort_unstable(); assert!(bank_snapshots .into_iter() @@ -511,6 +509,10 @@ mod tests { .unwrap() .take() .unwrap(); + solana_runtime::serde_snapshot::reserialize_bank( + accounts_package.snapshot_links.path(), + accounts_package.slot, + ); let snapshot_package = SnapshotPackage::from(accounts_package); pending_snapshot_package .lock() @@ -548,6 +550,9 @@ mod tests { ) .unwrap(); + // files were saved off before we reserialized the bank in the hacked up accounts_hash_verifier stand-in. + solana_runtime::serde_snapshot::reserialize_bank(saved_snapshots_dir.path(), saved_slot); + snapshot_utils::verify_snapshot_archive( saved_archive_path.unwrap(), saved_snapshots_dir.path(), @@ -751,7 +756,7 @@ mod tests { let slot = bank.slot(); info!("Making full snapshot archive from bank at slot: {}", slot); let bank_snapshot_info = - snapshot_utils::get_bank_snapshots(&snapshot_config.bank_snapshots_dir) + snapshot_utils::get_bank_snapshots_pre(&snapshot_config.bank_snapshots_dir) .into_iter() .find(|elem| elem.slot == slot) .ok_or_else(|| { @@ -786,7 +791,7 @@ mod tests { slot, incremental_snapshot_base_slot, ); let bank_snapshot_info = - snapshot_utils::get_bank_snapshots(&snapshot_config.bank_snapshots_dir) + snapshot_utils::get_bank_snapshots_pre(&snapshot_config.bank_snapshots_dir) .into_iter() .find(|elem| elem.slot == slot) .ok_or_else(|| { diff --git a/runtime/src/serde_snapshot.rs b/runtime/src/serde_snapshot.rs index b2666529e4..956af7a836 100644 --- a/runtime/src/serde_snapshot.rs +++ b/runtime/src/serde_snapshot.rs @@ -14,6 +14,7 @@ use { epoch_stakes::EpochStakes, hardened_unpack::UnpackedAppendVecMap, rent_collector::RentCollector, + snapshot_utils::{self, BANK_SNAPSHOT_PRE_FILENAME_EXTENSION}, stakes::Stakes, }, bincode::{self, config::Options, Error}, @@ -285,6 +286,21 @@ where }) } +pub fn reserialize_bank(bank_snapshots_dir: impl AsRef, slot: Slot) { + let bank_post = snapshot_utils::get_bank_snapshots_dir(bank_snapshots_dir, slot); + let bank_post = bank_post.join(snapshot_utils::get_snapshot_file_name(slot)); + let mut bank_pre = bank_post.clone(); + bank_pre.set_extension(BANK_SNAPSHOT_PRE_FILENAME_EXTENSION); + + // some tests don't create the file + if bank_pre.is_file() { + // replace the original file with the newly serialized one + // atm, this just copies the file and deletes the old one + std::fs::copy(&bank_pre, bank_post).unwrap(); + std::fs::remove_file(bank_pre).unwrap(); + } +} + struct SerializableBankAndStorage<'a, C> { bank: &'a Bank, snapshot_storages: &'a [SnapshotStorage], diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 439cb62f2a..4a701a0a3d 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -54,6 +54,7 @@ const MAX_SNAPSHOT_VERSION_FILE_SIZE: u64 = 8; // byte const VERSION_STRING_V1_2_0: &str = "1.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 BANK_SNAPSHOT_PRE_FILENAME_EXTENSION: &str = "pre"; 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; @@ -115,11 +116,16 @@ impl SnapshotVersion { } } -/// A slot and the path to its bank snapshot +/// Information about a bank snapshot. Namely the slot of the bank, the path to the snapshot, and +/// the type of the snapshot. #[derive(PartialEq, Eq, Debug)] pub struct BankSnapshotInfo { + /// Slot of the bank pub slot: Slot, + /// Path to the snapshot pub snapshot_path: PathBuf, + /// Type of the snapshot + pub snapshot_type: BankSnapshotType, } impl PartialOrd for BankSnapshotInfo { @@ -135,6 +141,22 @@ impl Ord for BankSnapshotInfo { } } +/// Bank snapshots traditionally had their accounts hash calculated prior to serialization. Since +/// the hash calculation takes a long time, an optimization has been put in to offload the accounts +/// hash calculation. The bank serialization format has not changed, so we need another way to +/// identify if a bank snapshot contains the calculated accounts hash or not. +/// +/// When a bank snapshot is first taken, it does not have the calculated accounts hash. It is said +/// that this bank snapshot is "pre" accounts hash. Later, when the accounts hash is calculated, +/// the bank snapshot is re-serialized, and is now "post" accounts hash. +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +pub enum BankSnapshotType { + /// This bank snapshot has *not* yet had its accounts hash calculated + Pre, + /// This bank snapshot *has* had its accounts hash calculated + Post, +} + /// Helper type when rebuilding from snapshots. Designed to handle when rebuilding from just a /// full snapshot, or from both a full snapshot and an incremental snapshot. #[derive(Debug)] @@ -388,54 +410,102 @@ pub fn archive_snapshot_package( Ok(()) } -/// Get a list of bank snapshots in a directory -pub fn get_bank_snapshots

(bank_snapshots_dir: P) -> Vec -where - P: AsRef, -{ +/// Get the bank snapshots in a directory +pub fn get_bank_snapshots(bank_snapshots_dir: impl AsRef) -> Vec { + let mut bank_snapshots = Vec::default(); match fs::read_dir(&bank_snapshots_dir) { - Ok(paths) => paths - .filter_map(|entry| { - entry.ok().and_then(|e| { - e.path() - .file_name() - .and_then(|n| n.to_str().map(|s| s.parse::().ok())) - .unwrap_or(None) - }) - }) - .map(|slot| { - let snapshot_file_name = get_snapshot_file_name(slot); - // So nice I join-ed it twice! The redundant `snapshot_file_name` is unintentional - // and should be simplified. Kept for compatibility. - let snapshot_path = bank_snapshots_dir - .as_ref() - .join(&snapshot_file_name) - .join(snapshot_file_name); - BankSnapshotInfo { - slot, - snapshot_path, - } - }) - .collect::>(), Err(err) => { info!( - "Unable to read snapshots directory {}: {}", + "Unable to read bank snapshots directory {}: {}", bank_snapshots_dir.as_ref().display(), err ); - vec![] } + Ok(paths) => paths + .filter_map(|entry| { + // check if this entry is a directory and only a Slot + // bank snapshots are bank_snapshots_dir/slot/slot(BANK_SNAPSHOT_PRE_FILENAME_EXTENSION) + entry + .ok() + .filter(|entry| entry.path().is_dir()) + .and_then(|entry| { + entry + .path() + .file_name() + .and_then(|file_name| file_name.to_str()) + .and_then(|file_name| file_name.parse::().ok()) + }) + }) + .for_each(|slot| { + // check this directory to see if there is a BankSnapshotPre and/or + // BankSnapshotPost file + let bank_snapshot_outer_dir = get_bank_snapshots_dir(&bank_snapshots_dir, slot); + let bank_snapshot_post_path = + bank_snapshot_outer_dir.join(get_snapshot_file_name(slot)); + let mut bank_snapshot_pre_path = bank_snapshot_post_path.clone(); + bank_snapshot_pre_path.set_extension(BANK_SNAPSHOT_PRE_FILENAME_EXTENSION); + + if bank_snapshot_pre_path.is_file() { + bank_snapshots.push(BankSnapshotInfo { + slot, + snapshot_path: bank_snapshot_pre_path, + snapshot_type: BankSnapshotType::Pre, + }); + } + + if bank_snapshot_post_path.is_file() { + bank_snapshots.push(BankSnapshotInfo { + slot, + snapshot_path: bank_snapshot_post_path, + snapshot_type: BankSnapshotType::Post, + }); + } + }), } + bank_snapshots +} + +/// Get the bank snapshots in a directory +/// +/// This function retains only the bank snapshots of type BankSnapshotType::Pre +pub fn get_bank_snapshots_pre(bank_snapshots_dir: impl AsRef) -> Vec { + let mut bank_snapshots = get_bank_snapshots(bank_snapshots_dir); + bank_snapshots.retain(|bank_snapshot| bank_snapshot.snapshot_type == BankSnapshotType::Pre); + bank_snapshots +} + +/// Get the bank snapshots in a directory +/// +/// This function retains only the bank snapshots of type BankSnapshotType::Post +pub fn get_bank_snapshots_post(bank_snapshots_dir: impl AsRef) -> Vec { + let mut bank_snapshots = get_bank_snapshots(bank_snapshots_dir); + bank_snapshots.retain(|bank_snapshot| bank_snapshot.snapshot_type == BankSnapshotType::Post); + bank_snapshots } /// Get the bank snapshot with the highest slot in a directory -pub fn get_highest_bank_snapshot_info

(bank_snapshots_dir: P) -> Option -where - P: AsRef, -{ - let mut bank_snapshot_infos = get_bank_snapshots(bank_snapshots_dir); - bank_snapshot_infos.sort_unstable(); - bank_snapshot_infos.into_iter().rev().next() +/// +/// This function gets the highest bank snapshot of type BankSnapshotType::Pre +pub fn get_highest_bank_snapshot_pre( + bank_snapshots_dir: impl AsRef, +) -> Option { + do_get_highest_bank_snapshot(get_bank_snapshots_pre(bank_snapshots_dir)) +} + +/// Get the bank snapshot with the highest slot in a directory +/// +/// This function gets the highest bank snapshot of type BankSnapshotType::Post +pub fn get_highest_bank_snapshot_post( + bank_snapshots_dir: impl AsRef, +) -> Option { + do_get_highest_bank_snapshot(get_bank_snapshots_post(bank_snapshots_dir)) +} + +fn do_get_highest_bank_snapshot( + mut bank_snapshots: Vec, +) -> Option { + bank_snapshots.sort_unstable(); + bank_snapshots.into_iter().rev().next() } pub fn serialize_snapshot_data_file(data_file_path: &Path, serializer: F) -> Result @@ -619,11 +689,14 @@ pub fn add_bank_snapshot>( let bank_snapshots_dir = get_bank_snapshots_dir(bank_snapshots_dir, slot); fs::create_dir_all(&bank_snapshots_dir)?; - // the bank snapshot is stored as bank_snapshots_dir/slot/slot - let snapshot_bank_file_path = bank_snapshots_dir.join(get_snapshot_file_name(slot)); + // the bank snapshot is stored as bank_snapshots_dir/slot/slot.BANK_SNAPSHOT_PRE_FILENAME_EXTENSION + let mut bank_snapshot_path = bank_snapshots_dir.join(get_snapshot_file_name(slot)); + bank_snapshot_path.set_extension(BANK_SNAPSHOT_PRE_FILENAME_EXTENSION); + info!( - "Creating snapshot for slot {}, path: {:?}", - slot, snapshot_bank_file_path, + "Creating bank snapshot for slot {}, path: {}", + slot, + bank_snapshot_path.display(), ); let mut bank_serialize = Measure::start("bank-serialize-ms"); @@ -635,7 +708,7 @@ pub fn add_bank_snapshot>( Ok(()) }; let consumed_size = - serialize_snapshot_data_file(&snapshot_bank_file_path, bank_snapshot_serializer)?; + serialize_snapshot_data_file(&bank_snapshot_path, bank_snapshot_serializer)?; bank_serialize.stop(); // Monitor sizes because they're capped to MAX_SNAPSHOT_DATA_FILE_SIZE @@ -648,13 +721,16 @@ pub fn add_bank_snapshot>( inc_new_counter_info!("bank-serialize-ms", bank_serialize.as_ms() as usize); info!( - "{} for slot {} at {:?}", - bank_serialize, slot, snapshot_bank_file_path, + "{} for slot {} at {}", + bank_serialize, + slot, + bank_snapshot_path.display(), ); Ok(BankSnapshotInfo { slot, - snapshot_path: snapshot_bank_file_path, + snapshot_path: bank_snapshot_path, + snapshot_type: BankSnapshotType::Pre, }) } @@ -1442,13 +1518,13 @@ fn verify_unpacked_snapshots_dir_and_version( &unpacked_snapshots_dir_and_version.snapshot_version, )) })?; - let mut bank_snapshot_infos = - get_bank_snapshots(&unpacked_snapshots_dir_and_version.unpacked_snapshots_dir); - if bank_snapshot_infos.len() > 1 { + let mut bank_snapshots = + get_bank_snapshots_post(&unpacked_snapshots_dir_and_version.unpacked_snapshots_dir); + if bank_snapshots.len() > 1 { return Err(get_io_error("invalid snapshot format")); } - bank_snapshot_infos.sort_unstable(); - let root_paths = bank_snapshot_infos + bank_snapshots.sort_unstable(); + let root_paths = bank_snapshots .pop() .ok_or_else(|| get_io_error("No snapshots found in snapshots directory"))?; Ok((snapshot_version, root_paths)) @@ -1560,11 +1636,11 @@ fn rebuild_bank_from_snapshots( Ok(bank) } -fn get_snapshot_file_name(slot: Slot) -> String { +pub(crate) fn get_snapshot_file_name(slot: Slot) -> String { slot.to_string() } -fn get_bank_snapshots_dir>(path: P, slot: Slot) -> PathBuf { +pub(crate) fn get_bank_snapshots_dir>(path: P, slot: Slot) -> PathBuf { path.as_ref().join(slot.to_string()) } @@ -1604,25 +1680,26 @@ pub fn verify_snapshot_archive( } /// Remove outdated bank snapshots -pub fn purge_old_bank_snapshots

(bank_snapshots_dir: P) -where - P: AsRef, -{ - let mut bank_snapshot_infos = get_bank_snapshots(&bank_snapshots_dir); - bank_snapshot_infos.sort_unstable(); - bank_snapshot_infos - .into_iter() - .rev() - .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 bank snapshot at: {}", - bank_snapshot_info.snapshot_path.display() - ); - } - }) +pub fn purge_old_bank_snapshots(bank_snapshots_dir: impl AsRef) { + let do_purge = |mut bank_snapshots: Vec| { + bank_snapshots.sort_unstable(); + bank_snapshots + .into_iter() + .rev() + .skip(MAX_BANK_SNAPSHOTS_TO_RETAIN) + .for_each(|bank_snapshot| { + let r = remove_bank_snapshot(bank_snapshot.slot, &bank_snapshots_dir); + if r.is_err() { + warn!( + "Couldn't remove bank snapshot at: {}", + bank_snapshot.snapshot_path.display() + ); + } + }) + }; + + do_purge(get_bank_snapshots_pre(&bank_snapshots_dir)); + do_purge(get_bank_snapshots_post(&bank_snapshots_dir)); } /// Gather the necessary elements for a snapshot of the given `root_bank`. @@ -1819,6 +1896,11 @@ pub fn package_and_archive_full_snapshot( Some(SnapshotType::FullSnapshot), )?; + crate::serde_snapshot::reserialize_bank( + accounts_package.snapshot_links.path(), + accounts_package.slot, + ); + let snapshot_package = SnapshotPackage::from(accounts_package); archive_snapshot_package( &snapshot_package, @@ -1860,6 +1942,11 @@ pub fn package_and_archive_incremental_snapshot( )), )?; + crate::serde_snapshot::reserialize_bank( + accounts_package.snapshot_links.path(), + accounts_package.slot, + ); + let snapshot_package = SnapshotPackage::from(accounts_package); archive_snapshot_package( &snapshot_package, @@ -2279,28 +2366,28 @@ mod tests { } #[test] - fn test_get_bank_snapshot_infos() { + fn test_get_bank_snapshots() { solana_logger::setup(); let temp_snapshots_dir = tempfile::TempDir::new().unwrap(); let min_slot = 10; let max_slot = 20; common_create_bank_snapshot_files(temp_snapshots_dir.path(), min_slot, max_slot); - let bank_snapshot_infos = get_bank_snapshots(temp_snapshots_dir.path()); - assert_eq!(bank_snapshot_infos.len() as Slot, max_slot - min_slot); + let bank_snapshots = get_bank_snapshots(temp_snapshots_dir.path()); + assert_eq!(bank_snapshots.len() as Slot, max_slot - min_slot); } #[test] - fn test_get_highest_bank_snapshot_info() { + fn test_get_highest_bank_snapshot_post() { solana_logger::setup(); let temp_snapshots_dir = tempfile::TempDir::new().unwrap(); let min_slot = 99; let max_slot = 123; common_create_bank_snapshot_files(temp_snapshots_dir.path(), min_slot, max_slot); - let highest_bank_snapshot_info = get_highest_bank_snapshot_info(temp_snapshots_dir.path()); - assert!(highest_bank_snapshot_info.is_some()); - assert_eq!(highest_bank_snapshot_info.unwrap().slot, max_slot - 1); + let highest_bank_snapshot = get_highest_bank_snapshot_post(temp_snapshots_dir.path()); + assert!(highest_bank_snapshot.is_some()); + assert_eq!(highest_bank_snapshot.unwrap().slot, max_slot - 1); } /// A test helper function that creates full and incremental snapshot archive files. Creates