From def0a86e65c7e5f6aa283566b2906ee3fab2cae5 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Thu, 16 Feb 2023 09:45:46 -0800 Subject: [PATCH 01/12] Read snapshot directories and find the highest --- runtime/src/snapshot_utils.rs | 150 +++++++++++++++++++++++++++++----- 1 file changed, 130 insertions(+), 20 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 919ac647b6605b..9b595aaba56ed2 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -143,6 +143,8 @@ pub struct BankSnapshotInfo { pub snapshot_type: BankSnapshotType, /// Path to the bank snapshot directory pub snapshot_dir: PathBuf, + /// Snapshot version + pub snapshot_version: SnapshotVersion, } impl PartialOrd for BankSnapshotInfo { @@ -162,31 +164,52 @@ impl BankSnapshotInfo { pub fn new_from_dir( bank_snapshots_dir: impl AsRef, slot: Slot, - ) -> Option { + ) -> Result { // check this directory to see if there is a BankSnapshotPre and/or // BankSnapshotPost file let bank_snapshot_dir = get_bank_snapshots_dir(&bank_snapshots_dir, slot); - let bank_snapshot_post_path = bank_snapshot_dir.join(get_snapshot_file_name(slot)); - let bank_snapshot_pre_path = - bank_snapshot_post_path.with_extension(BANK_SNAPSHOT_PRE_FILENAME_EXTENSION); - if bank_snapshot_pre_path.is_file() { - return Some(BankSnapshotInfo { - slot, - snapshot_type: BankSnapshotType::Pre, - snapshot_dir: bank_snapshot_dir, - }); + if !bank_snapshot_dir.is_dir() { + return Err(SnapshotError::InvalidBankSnapshotDir(bank_snapshot_dir)); } - if bank_snapshot_post_path.is_file() { - return Some(BankSnapshotInfo { - slot, - snapshot_type: BankSnapshotType::Post, - snapshot_dir: bank_snapshot_dir, - }); + let status_cache_file = bank_snapshot_dir.join(SNAPSHOT_STATUS_CACHE_FILENAME); + if fs::metadata(status_cache_file).is_err() { + return Err(SnapshotError::MissingStatusCacheFile(bank_snapshot_dir)); } - None + let version_path = bank_snapshot_dir.join(SNAPSHOT_VERSION_FILENAME); + let version_str = snapshot_version_from_file(version_path)?; + let snapshot_version = SnapshotVersion::from_str(version_str.as_str()) + .or(Err(SnapshotError::InvalidVersion))?; + + // There is a time window from the slot directory being created, and the content being completely + // filled. Check the completion to avoid using a highest found slot directory with missing content. + let completion_flag_file = bank_snapshot_dir.join(SNAPSHOT_STATE_COMPLETE_FILENAME); + if fs::metadata(completion_flag_file).is_err() { + return Err(SnapshotError::DirIncomplete(bank_snapshot_dir)); + } + + let bank_snapshot_post_path = bank_snapshot_dir.join(get_snapshot_file_name(slot)); + let bank_snapshot_pre_path = + bank_snapshot_post_path.with_extension(BANK_SNAPSHOT_PRE_FILENAME_EXTENSION); + + let snapshot_type: Option = if bank_snapshot_pre_path.is_file() { + Some(BankSnapshotType::Pre) + } else if bank_snapshot_post_path.is_file() { + Some(BankSnapshotType::Post) + } else { + None + }; + let snapshot_type = snapshot_type + .ok_or_else(|| SnapshotError::MissingSnapshotFile(bank_snapshot_dir.clone()))?; + + Ok(BankSnapshotInfo { + slot, + snapshot_type, + snapshot_dir: bank_snapshot_dir, + snapshot_version, + }) } pub fn snapshot_path(&self) -> PathBuf { @@ -299,6 +322,24 @@ pub enum SnapshotError { #[error("invalid account path: {}", .0.display())] InvalidAccountPath(PathBuf), + + #[error("no valid snapshot dir found under {}", .0.display())] + NoSnapshotSlotDir(PathBuf), + + #[error("invalid bank snapshot directory {}", .0.display())] + InvalidBankSnapshotDir(PathBuf), + + #[error("missing status cache file {}", .0.display())] + MissingStatusCacheFile(PathBuf), + + #[error("invalid snapshot version")] + InvalidVersion, + + #[error("snapshot directory incomplete {}", .0.display())] + DirIncomplete(PathBuf), + + #[error("missing snapshotfile {}", .0.display())] + MissingSnapshotFile(PathBuf), } pub type Result = std::result::Result; @@ -645,8 +686,7 @@ pub fn get_bank_snapshots(bank_snapshots_dir: impl AsRef) -> Vec) -> Result { + let bank_snapshots = get_bank_snapshots(&snapshots_dir); + + do_get_highest_bank_snapshot(bank_snapshots).ok_or_else(|| SnapshotError::NoSnapshotSlotDir( + snapshots_dir.as_ref().to_path_buf(), + )) +} + +pub fn get_highest_incremental_snapshot( + _snapshots_dir: impl AsRef, + _full_snapshot_slot: Slot, +) -> Option { + None // Will handle the incremental case later +} + /// Get the path (and metadata) for the full snapshot archive with the highest slot in a directory pub fn get_highest_full_snapshot_archive_info( full_snapshot_archives_dir: impl AsRef, @@ -2685,7 +2744,10 @@ pub fn create_tmp_accounts_dir_for_tests() -> (TempDir, PathBuf) { mod tests { use { super::*, - crate::{accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING, status_cache::Status}, + crate::{ + accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING, bank_forks::BankForks, + status_cache::Status, + }, assert_matches::assert_matches, bincode::{deserialize_from, serialize_into}, solana_sdk::{ @@ -4522,4 +4584,52 @@ mod tests { assert!(matches!(ret, Err(SnapshotError::InvalidAppendVecPath(_)))); } + + #[test] + fn test_get_highest_full_snapshot() { + solana_logger::setup(); + + let genesis_config = GenesisConfig::default(); + let bank0 = Bank::new_for_tests(&genesis_config); + let mut bank_forks = BankForks::new(bank0); + + let tmp_dir = tempfile::TempDir::new().unwrap(); + let bank_snapshots_dir = tmp_dir.path(); + let collecter_id = Pubkey::new_unique(); + let snapshot_version = SnapshotVersion::default(); + + for slot in 0..4 { + // prepare the bank + let parent_bank = bank_forks.get(slot).unwrap(); + let bank = Bank::new_from_parent(&parent_bank, &collecter_id, slot + 1); + bank.fill_bank_with_ticks_for_tests(); + bank.squash(); + bank.force_flush_accounts_cache(); + + // generate the bank snapshot directory for slot+1 + let snapshot_storages = bank.get_snapshot_storages(None); + let slot_deltas = bank.status_cache.read().unwrap().root_slot_deltas(); + add_bank_snapshot( + bank_snapshots_dir, + &bank, + &snapshot_storages, + snapshot_version, + slot_deltas, + ) + .unwrap(); + + bank_forks.insert(bank); + } + + let snapshot = get_highest_full_snapshot(bank_snapshots_dir).unwrap(); + + assert_eq!(snapshot.slot, 4); + + let complete_flag_file = snapshot.snapshot_dir.join(SNAPSHOT_STATE_COMPLETE_FILENAME); + fs::remove_file(complete_flag_file).unwrap(); + + let snapshot = get_highest_full_snapshot(bank_snapshots_dir).unwrap(); + + assert_eq!(snapshot.slot, 3); + } } From c5bb04fb5e8fed0cdd70bea3b40fdee5e4ce1630 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Thu, 16 Feb 2023 12:03:18 -0800 Subject: [PATCH 02/12] format check --- runtime/src/snapshot_utils.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 9b595aaba56ed2..447dfa2f8ece81 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -1887,9 +1887,8 @@ pub fn get_highest_incremental_snapshot_archive_slot( pub fn get_highest_full_snapshot(snapshots_dir: impl AsRef) -> Result { let bank_snapshots = get_bank_snapshots(&snapshots_dir); - do_get_highest_bank_snapshot(bank_snapshots).ok_or_else(|| SnapshotError::NoSnapshotSlotDir( - snapshots_dir.as_ref().to_path_buf(), - )) + do_get_highest_bank_snapshot(bank_snapshots) + .ok_or_else(|| SnapshotError::NoSnapshotSlotDir(snapshots_dir.as_ref().to_path_buf())) } pub fn get_highest_incremental_snapshot( From a46e77ad048cc31d40d642dbe622e2210b124e9f Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Thu, 16 Feb 2023 13:48:41 -0800 Subject: [PATCH 03/12] Fix test_get_bank_snapshots --- runtime/src/snapshot_utils.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 447dfa2f8ece81..05767890963be6 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -3130,6 +3130,16 @@ mod tests { let snapshot_filename = get_snapshot_file_name(slot); let snapshot_path = snapshot_dir.join(snapshot_filename); File::create(snapshot_path).unwrap(); + + let status_cache_file = snapshot_dir.join(SNAPSHOT_STATUS_CACHE_FILENAME); + File::create(status_cache_file).unwrap(); + + let version_path = snapshot_dir.join(SNAPSHOT_VERSION_FILENAME); + write_snapshot_version_file(version_path, SnapshotVersion::default()).unwrap(); + + // Mark this directory complete so it can be used. Check this flag first before selecting for deserialization. + let state_complete_path = snapshot_dir.join(SNAPSHOT_STATE_COMPLETE_FILENAME); + fs::File::create(state_complete_path).unwrap(); } } From 7eb6ed74bcf61e33231c6f3ff5bb5c2e46e9f386 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Thu, 16 Feb 2023 21:45:53 -0800 Subject: [PATCH 04/12] fix test_bank_fields_from_snapshot --- runtime/src/snapshot_utils.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 05767890963be6..13d2f718b27241 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -1589,6 +1589,39 @@ fn streaming_unarchive_snapshot( .collect() } +/// BankSnapshotInfo::new_from_dir() requires a few meta files to accept a snapshot dir +/// as a valid one. A dir unpacked from an archive lacks these files. Fill them here to +/// allow new_from_dir() checks to pass. These checks are not needed for unpacked dirs, +/// but it is not clean to add another flag to new_from_dir() to skip them. +fn fill_snapshot_meta_files_for_unchived_snapshot(unpack_dir: impl AsRef) -> Result<()> { + let snapshots_dir = unpack_dir.as_ref().join("snapshots"); + if !snapshots_dir.is_dir() { + return Err(SnapshotError::NoSnapshotSlotDir(snapshots_dir)); + } + + // The unpacked dir has a single slot dir, which is the snapshot slot dir. + let slot_dir = fs::read_dir(&snapshots_dir) + .unwrap() + .find(|entry| entry.as_ref().unwrap().path().is_dir()) + .unwrap() + .unwrap() + .path(); + + let version_file = unpack_dir.as_ref().join(SNAPSHOT_VERSION_FILENAME); + fs::hard_link(version_file, slot_dir.join(SNAPSHOT_VERSION_FILENAME))?; + + let status_cache_file = snapshots_dir.join(SNAPSHOT_STATUS_CACHE_FILENAME); + fs::hard_link( + status_cache_file, + slot_dir.join(SNAPSHOT_STATUS_CACHE_FILENAME), + )?; + + let state_complete_file = slot_dir.join(SNAPSHOT_STATE_COMPLETE_FILENAME); + fs::File::create(state_complete_file)?; + + Ok(()) +} + /// Perform the common tasks when unarchiving a snapshot. Handles creating the temporary /// directories, untaring, reading the version file, and then returning those fields plus the /// rebuilt storage @@ -1634,6 +1667,8 @@ where ); info!("{}", measure_untar); + fill_snapshot_meta_files_for_unchived_snapshot(&unpack_dir)?; + let RebuiltSnapshotStorage { snapshot_version, storage, From b8f24cd1d597e4e0267c2b355a5507ae870240a1 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Fri, 17 Feb 2023 11:38:44 -0800 Subject: [PATCH 05/12] review changes, is_file etc --- runtime/src/snapshot_utils.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 13d2f718b27241..0ba7789bebb19b 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -174,8 +174,8 @@ impl BankSnapshotInfo { } let status_cache_file = bank_snapshot_dir.join(SNAPSHOT_STATUS_CACHE_FILENAME); - if fs::metadata(status_cache_file).is_err() { - return Err(SnapshotError::MissingStatusCacheFile(bank_snapshot_dir)); + if !fs::metadata(&status_cache_file)?.is_file() { + return Err(SnapshotError::MissingStatusCacheFile(status_cache_file)); } let version_path = bank_snapshot_dir.join(SNAPSHOT_VERSION_FILENAME); @@ -186,23 +186,23 @@ impl BankSnapshotInfo { // There is a time window from the slot directory being created, and the content being completely // filled. Check the completion to avoid using a highest found slot directory with missing content. let completion_flag_file = bank_snapshot_dir.join(SNAPSHOT_STATE_COMPLETE_FILENAME); - if fs::metadata(completion_flag_file).is_err() { - return Err(SnapshotError::DirIncomplete(bank_snapshot_dir)); + if !fs::metadata(&completion_flag_file)?.is_file() { + return Err(SnapshotError::DirIncomplete(completion_flag_file)); } let bank_snapshot_post_path = bank_snapshot_dir.join(get_snapshot_file_name(slot)); let bank_snapshot_pre_path = bank_snapshot_post_path.with_extension(BANK_SNAPSHOT_PRE_FILENAME_EXTENSION); - let snapshot_type: Option = if bank_snapshot_pre_path.is_file() { - Some(BankSnapshotType::Pre) + let snapshot_type = if bank_snapshot_pre_path.is_file() { + BankSnapshotType::Pre } else if bank_snapshot_post_path.is_file() { - Some(BankSnapshotType::Post) + BankSnapshotType::Post } else { - None + return Err(SnapshotError::MissingSnapshotFile( + bank_snapshot_dir.clone(), + )); }; - let snapshot_type = snapshot_type - .ok_or_else(|| SnapshotError::MissingSnapshotFile(bank_snapshot_dir.clone()))?; Ok(BankSnapshotInfo { slot, @@ -1593,7 +1593,7 @@ fn streaming_unarchive_snapshot( /// as a valid one. A dir unpacked from an archive lacks these files. Fill them here to /// allow new_from_dir() checks to pass. These checks are not needed for unpacked dirs, /// but it is not clean to add another flag to new_from_dir() to skip them. -fn fill_snapshot_meta_files_for_unchived_snapshot(unpack_dir: impl AsRef) -> Result<()> { +fn fill_snapshot_meta_files_for_unarchived_snapshot(unpack_dir: impl AsRef) -> Result<()> { let snapshots_dir = unpack_dir.as_ref().join("snapshots"); if !snapshots_dir.is_dir() { return Err(SnapshotError::NoSnapshotSlotDir(snapshots_dir)); @@ -1667,7 +1667,7 @@ where ); info!("{}", measure_untar); - fill_snapshot_meta_files_for_unchived_snapshot(&unpack_dir)?; + fill_snapshot_meta_files_for_unarchived_snapshot(&unpack_dir)?; let RebuiltSnapshotStorage { snapshot_version, From 3dd4c7221c35cafab7da9c47a13a6b82d56b78f7 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Fri, 17 Feb 2023 12:26:17 -0800 Subject: [PATCH 06/12] removed incremental snapshot --- runtime/src/snapshot_utils.rs | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 0ba7789bebb19b..12495921ecfa2b 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -199,9 +199,7 @@ impl BankSnapshotInfo { } else if bank_snapshot_post_path.is_file() { BankSnapshotType::Post } else { - return Err(SnapshotError::MissingSnapshotFile( - bank_snapshot_dir.clone(), - )); + return Err(SnapshotError::MissingSnapshotFile(bank_snapshot_dir)); }; Ok(BankSnapshotInfo { @@ -1919,20 +1917,12 @@ pub fn get_highest_incremental_snapshot_archive_slot( /// Get the full snapshot with the highest slot in a directory /// Assume every bank snapshot directory is a full snapshot for now. Will support incremental sanpshot /// later. -pub fn get_highest_full_snapshot(snapshots_dir: impl AsRef) -> Result { +pub fn get_highest_bank_snapshot(snapshots_dir: impl AsRef) -> Result { let bank_snapshots = get_bank_snapshots(&snapshots_dir); - do_get_highest_bank_snapshot(bank_snapshots) .ok_or_else(|| SnapshotError::NoSnapshotSlotDir(snapshots_dir.as_ref().to_path_buf())) } -pub fn get_highest_incremental_snapshot( - _snapshots_dir: impl AsRef, - _full_snapshot_slot: Slot, -) -> Option { - None // Will handle the incremental case later -} - /// Get the path (and metadata) for the full snapshot archive with the highest slot in a directory pub fn get_highest_full_snapshot_archive_info( full_snapshot_archives_dir: impl AsRef, @@ -4630,7 +4620,7 @@ mod tests { } #[test] - fn test_get_highest_full_snapshot() { + fn test_get_highest_bank_snapshot() { solana_logger::setup(); let genesis_config = GenesisConfig::default(); @@ -4665,15 +4655,22 @@ mod tests { bank_forks.insert(bank); } - let snapshot = get_highest_full_snapshot(bank_snapshots_dir).unwrap(); - + let snapshot = get_highest_bank_snapshot(bank_snapshots_dir).unwrap(); assert_eq!(snapshot.slot, 4); let complete_flag_file = snapshot.snapshot_dir.join(SNAPSHOT_STATE_COMPLETE_FILENAME); fs::remove_file(complete_flag_file).unwrap(); + let snapshot = get_highest_bank_snapshot(bank_snapshots_dir).unwrap(); + assert_eq!(snapshot.slot, 3); - let snapshot = get_highest_full_snapshot(bank_snapshots_dir).unwrap(); + let snapshot_version_file = snapshot.snapshot_dir.join(SNAPSHOT_VERSION_FILENAME); + fs::remove_file(snapshot_version_file).unwrap(); + let snapshot = get_highest_bank_snapshot(bank_snapshots_dir).unwrap(); + assert_eq!(snapshot.slot, 2); - assert_eq!(snapshot.slot, 3); + let status_cache_file = snapshot.snapshot_dir.join(SNAPSHOT_STATUS_CACHE_FILENAME); + fs::remove_file(status_cache_file).unwrap(); + let snapshot = get_highest_bank_snapshot(bank_snapshots_dir).unwrap(); + assert_eq!(snapshot.slot, 1); } } From f8e25f98ca9ef55038a4f143d5e974ea33876919 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Fri, 17 Feb 2023 18:35:59 -0800 Subject: [PATCH 07/12] SnapshotNewFromDirError --- runtime/src/snapshot_utils.rs | 47 +++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 12495921ecfa2b..62a020506e913c 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -164,30 +164,36 @@ impl BankSnapshotInfo { pub fn new_from_dir( bank_snapshots_dir: impl AsRef, slot: Slot, - ) -> Result { + ) -> std::result::Result { // check this directory to see if there is a BankSnapshotPre and/or // BankSnapshotPost file let bank_snapshot_dir = get_bank_snapshots_dir(&bank_snapshots_dir, slot); if !bank_snapshot_dir.is_dir() { - return Err(SnapshotError::InvalidBankSnapshotDir(bank_snapshot_dir)); + return Err(SnapshotNewFromDirError::InvalidBankSnapshotDir( + bank_snapshot_dir, + )); } let status_cache_file = bank_snapshot_dir.join(SNAPSHOT_STATUS_CACHE_FILENAME); if !fs::metadata(&status_cache_file)?.is_file() { - return Err(SnapshotError::MissingStatusCacheFile(status_cache_file)); + return Err(SnapshotNewFromDirError::MissingStatusCacheFile( + status_cache_file, + )); } let version_path = bank_snapshot_dir.join(SNAPSHOT_VERSION_FILENAME); - let version_str = snapshot_version_from_file(version_path)?; + let version_str = snapshot_version_from_file(&version_path).or(Err( + SnapshotNewFromDirError::MissingVersionFile(version_path), + ))?; let snapshot_version = SnapshotVersion::from_str(version_str.as_str()) - .or(Err(SnapshotError::InvalidVersion))?; + .or(Err(SnapshotNewFromDirError::InvalidVersion))?; // There is a time window from the slot directory being created, and the content being completely // filled. Check the completion to avoid using a highest found slot directory with missing content. let completion_flag_file = bank_snapshot_dir.join(SNAPSHOT_STATE_COMPLETE_FILENAME); if !fs::metadata(&completion_flag_file)?.is_file() { - return Err(SnapshotError::DirIncomplete(completion_flag_file)); + return Err(SnapshotNewFromDirError::DirIncomplete(completion_flag_file)); } let bank_snapshot_post_path = bank_snapshot_dir.join(get_snapshot_file_name(slot)); @@ -199,7 +205,9 @@ impl BankSnapshotInfo { } else if bank_snapshot_post_path.is_file() { BankSnapshotType::Post } else { - return Err(SnapshotError::MissingSnapshotFile(bank_snapshot_dir)); + return Err(SnapshotNewFromDirError::MissingSnapshotFile( + bank_snapshot_dir, + )); }; Ok(BankSnapshotInfo { @@ -323,6 +331,11 @@ pub enum SnapshotError { #[error("no valid snapshot dir found under {}", .0.display())] NoSnapshotSlotDir(PathBuf), +} +#[derive(Error, Debug)] +pub enum SnapshotNewFromDirError { + #[error("I/O error: {0}")] + Io(#[from] std::io::Error), #[error("invalid bank snapshot directory {}", .0.display())] InvalidBankSnapshotDir(PathBuf), @@ -330,6 +343,9 @@ pub enum SnapshotError { #[error("missing status cache file {}", .0.display())] MissingStatusCacheFile(PathBuf), + #[error("missing version file {}", .0.display())] + MissingVersionFile(PathBuf), + #[error("invalid snapshot version")] InvalidVersion, @@ -339,6 +355,7 @@ pub enum SnapshotError { #[error("missing snapshotfile {}", .0.display())] MissingSnapshotFile(PathBuf), } + pub type Result = std::result::Result; /// Errors that can happen in `verify_slot_deltas()` @@ -683,12 +700,16 @@ pub fn get_bank_snapshots(bank_snapshots_dir: impl AsRef) -> Vec().ok()) }) }) - .for_each(|slot| { - if let Ok(snapshot_info) = BankSnapshotInfo::new_from_dir(&bank_snapshots_dir, slot) - { - bank_snapshots.push(snapshot_info); - } - }), + .for_each( + |slot| match BankSnapshotInfo::new_from_dir(&bank_snapshots_dir, slot) { + Ok(snapshot_info) => { + bank_snapshots.push(snapshot_info); + } + Err(err) => { + error!("Unable to read bank snapshot for slot {}: {}", slot, err); + } + }, + ), } bank_snapshots } From 7b8ada6afe2778463f0823a882ba56565c06beaf Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Tue, 21 Feb 2023 11:42:04 -0800 Subject: [PATCH 08/12] nit and comments issues --- runtime/src/snapshot_utils.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 62a020506e913c..1d99a7f8396e28 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -193,7 +193,7 @@ impl BankSnapshotInfo { // filled. Check the completion to avoid using a highest found slot directory with missing content. let completion_flag_file = bank_snapshot_dir.join(SNAPSHOT_STATE_COMPLETE_FILENAME); if !fs::metadata(&completion_flag_file)?.is_file() { - return Err(SnapshotNewFromDirError::DirIncomplete(completion_flag_file)); + return Err(SnapshotNewFromDirError::IncompleteDir(completion_flag_file)); } let bank_snapshot_post_path = bank_snapshot_dir.join(get_snapshot_file_name(slot)); @@ -350,9 +350,9 @@ pub enum SnapshotNewFromDirError { InvalidVersion, #[error("snapshot directory incomplete {}", .0.display())] - DirIncomplete(PathBuf), + IncompleteDir(PathBuf), - #[error("missing snapshotfile {}", .0.display())] + #[error("missing snapshot file {}", .0.display())] MissingSnapshotFile(PathBuf), } @@ -750,6 +750,13 @@ pub fn get_highest_bank_snapshot_post( do_get_highest_bank_snapshot(get_bank_snapshots_post(bank_snapshots_dir)) } +/// Get the bank snapshot with the highest slot in a directory +/// +/// This function gets the highest bank snapshot of any type +pub fn get_highest_bank_snapshot(bank_snapshots_dir: impl AsRef) -> Option { + do_get_highest_bank_snapshot(get_bank_snapshots(&bank_snapshots_dir)) +} + fn do_get_highest_bank_snapshot( mut bank_snapshots: Vec, ) -> Option { @@ -1935,15 +1942,6 @@ pub fn get_highest_incremental_snapshot_archive_slot( .map(|incremental_snapshot_archive_info| incremental_snapshot_archive_info.slot()) } -/// Get the full snapshot with the highest slot in a directory -/// Assume every bank snapshot directory is a full snapshot for now. Will support incremental sanpshot -/// later. -pub fn get_highest_bank_snapshot(snapshots_dir: impl AsRef) -> Result { - let bank_snapshots = get_bank_snapshots(&snapshots_dir); - do_get_highest_bank_snapshot(bank_snapshots) - .ok_or_else(|| SnapshotError::NoSnapshotSlotDir(snapshots_dir.as_ref().to_path_buf())) -} - /// Get the path (and metadata) for the full snapshot archive with the highest slot in a directory pub fn get_highest_full_snapshot_archive_info( full_snapshot_archives_dir: impl AsRef, From b326805868aa346d9e476aea18950c3bd454da34 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Wed, 22 Feb 2023 13:40:07 -0800 Subject: [PATCH 09/12] NewFromDir(#[from] SnapshotNewFromDirError) --- runtime/src/snapshot_utils.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 1d99a7f8396e28..4a39d649aaadc8 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -323,6 +323,9 @@ pub enum SnapshotError { #[error("snapshot slot deltas are invalid: {0}")] VerifySlotDeltas(#[from] VerifySlotDeltasError), + #[error("bank_snapshot_info new_from_dir failed: {0}")] + NewFromDir(#[from] SnapshotNewFromDirError), + #[error("invalid AppendVec path: {}", .0.display())] InvalidAppendVecPath(PathBuf), From febb5a2e36313ec8f9ca5af91c8f18f6528b6ba5 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Thu, 23 Feb 2023 17:42:12 -0800 Subject: [PATCH 10/12] change fill to create --- runtime/src/snapshot_utils.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 4a39d649aaadc8..7246044ed515ba 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -1622,7 +1622,7 @@ fn streaming_unarchive_snapshot( /// as a valid one. A dir unpacked from an archive lacks these files. Fill them here to /// allow new_from_dir() checks to pass. These checks are not needed for unpacked dirs, /// but it is not clean to add another flag to new_from_dir() to skip them. -fn fill_snapshot_meta_files_for_unarchived_snapshot(unpack_dir: impl AsRef) -> Result<()> { +fn create_snapshot_meta_files_for_unarchived_snapshot(unpack_dir: impl AsRef) -> Result<()> { let snapshots_dir = unpack_dir.as_ref().join("snapshots"); if !snapshots_dir.is_dir() { return Err(SnapshotError::NoSnapshotSlotDir(snapshots_dir)); @@ -1696,7 +1696,7 @@ where ); info!("{}", measure_untar); - fill_snapshot_meta_files_for_unarchived_snapshot(&unpack_dir)?; + create_snapshot_meta_files_for_unarchived_snapshot(&unpack_dir)?; let RebuiltSnapshotStorage { snapshot_version, From 291a30b7597fe8293b500cc2c81c6e23b74eb479 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Thu, 23 Feb 2023 18:57:26 -0800 Subject: [PATCH 11/12] replace unwrap with map_err and ok_or_else --- runtime/src/snapshot_utils.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 7246044ed515ba..9851e37dbac16c 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -1630,10 +1630,10 @@ fn create_snapshot_meta_files_for_unarchived_snapshot(unpack_dir: impl AsRef Date: Fri, 24 Feb 2023 09:42:43 -0800 Subject: [PATCH 12/12] Remove BankForks, fix the bank loop --- runtime/src/snapshot_utils.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 9851e37dbac16c..ad2aff2380e540 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -2790,10 +2790,7 @@ pub fn create_tmp_accounts_dir_for_tests() -> (TempDir, PathBuf) { mod tests { use { super::*, - crate::{ - accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING, bank_forks::BankForks, - status_cache::Status, - }, + crate::{accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING, status_cache::Status}, assert_matches::assert_matches, bincode::{deserialize_from, serialize_into}, solana_sdk::{ @@ -4646,18 +4643,16 @@ mod tests { solana_logger::setup(); let genesis_config = GenesisConfig::default(); - let bank0 = Bank::new_for_tests(&genesis_config); - let mut bank_forks = BankForks::new(bank0); + let mut bank = Arc::new(Bank::new_for_tests(&genesis_config)); let tmp_dir = tempfile::TempDir::new().unwrap(); let bank_snapshots_dir = tmp_dir.path(); let collecter_id = Pubkey::new_unique(); let snapshot_version = SnapshotVersion::default(); - for slot in 0..4 { + for _ in 0..4 { // prepare the bank - let parent_bank = bank_forks.get(slot).unwrap(); - let bank = Bank::new_from_parent(&parent_bank, &collecter_id, slot + 1); + bank = Arc::new(Bank::new_from_parent(&bank, &collecter_id, bank.slot() + 1)); bank.fill_bank_with_ticks_for_tests(); bank.squash(); bank.force_flush_accounts_cache(); @@ -4673,8 +4668,6 @@ mod tests { slot_deltas, ) .unwrap(); - - bank_forks.insert(bank); } let snapshot = get_highest_bank_snapshot(bank_snapshots_dir).unwrap();