Clean up the outdated SnapshotPackage snapshot_links field (#31360)

* Remove snapshot_links

* Change the function name from snapshot_dir to bank_snapshot_dir

* Format fix

* Fix test_concurrent_snapshot_packaging

* Fix clippy error

* Fix nits

* Fix nits 2nd try

* Use get_bank_snapshots_dir

* Use slot_dir

* Revert "Use get_bank_snapshots_dir" because get_bank_snapshots_dir is private to crate

This reverts commit 1ed9b3b2c8e84689a918beee7159f63c56500a96.
This commit is contained in:
Xiang Zhu 2023-05-01 11:24:59 -07:00 committed by GitHub
parent 58ce19446f
commit 0a2e897f16
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 26 additions and 33 deletions

View File

@ -297,7 +297,7 @@ impl AccountsHashVerifier {
if let Some(snapshot_info) = &accounts_package.snapshot_info {
solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash(
&snapshot_info.snapshot_links,
&snapshot_info.bank_snapshot_dir,
accounts_package.slot,
&accounts_hash_for_reserialize,
bank_incremental_snapshot_persistence.as_ref(),

View File

@ -285,7 +285,7 @@ mod tests {
archive_format: ArchiveFormat::Tar,
},
block_height: slot,
snapshot_links: PathBuf::default(),
bank_snapshot_dir: PathBuf::default(),
snapshot_storages: Vec::default(),
snapshot_version: SnapshotVersion::default(),
snapshot_type,

View File

@ -260,7 +260,7 @@ fn run_bank_forks_snapshot_n<F>(
let accounts_hash =
last_bank.update_accounts_hash(CalcAccountsHashDataSource::Storages, false, false);
solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash(
accounts_package.snapshot_links_dir(),
accounts_package.bank_snapshot_dir(),
accounts_package.slot,
&accounts_hash,
None,
@ -523,7 +523,7 @@ fn test_concurrent_snapshot_packaging(
let accounts_package = real_accounts_package_receiver.try_recv().unwrap();
let accounts_hash = AccountsHash(Hash::default());
solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash(
accounts_package.snapshot_links_dir(),
accounts_package.bank_snapshot_dir(),
accounts_package.slot,
&accounts_hash,
None,
@ -555,7 +555,7 @@ fn test_concurrent_snapshot_packaging(
// files were saved off before we reserialized the bank in the hacked up accounts_hash_verifier stand-in.
solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash(
saved_snapshots_dir.path(),
saved_snapshots_dir.path().join(saved_slot.to_string()),
saved_slot,
&AccountsHash(Hash::default()),
None,

View File

@ -455,13 +455,14 @@ where
/// write serialized bank to post file
/// return true if pre file found
pub fn reserialize_bank_with_new_accounts_hash(
bank_snapshots_dir: impl AsRef<Path>,
bank_snapshot_dir: impl AsRef<Path>,
slot: Slot,
accounts_hash: &AccountsHash,
incremental_snapshot_persistence: Option<&BankIncrementalSnapshotPersistence>,
) -> bool {
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 bank_post = bank_snapshot_dir
.as_ref()
.join(snapshot_utils::get_snapshot_file_name(slot));
let bank_pre = bank_post.with_extension(BANK_SNAPSHOT_PRE_FILENAME_EXTENSION);
let mut found = false;

View File

@ -320,7 +320,7 @@ fn test_bank_serialize_style(
if reserialize_accounts_hash || incremental_snapshot_persistence {
let temp_dir = TempDir::new().unwrap();
let slot_dir = temp_dir.path().join(slot.to_string());
let slot_dir = snapshot_utils::get_bank_snapshots_dir(&temp_dir, slot);
let post_path = slot_dir.join(slot.to_string());
let pre_path = post_path.with_extension(BANK_SNAPSHOT_PRE_FILENAME_EXTENSION);
std::fs::create_dir(&slot_dir).unwrap();
@ -330,7 +330,7 @@ fn test_bank_serialize_style(
}
assert!(reserialize_bank_with_new_accounts_hash(
temp_dir.path(),
slot_dir,
slot,
&accounts_hash,
incremental.as_ref(),

View File

@ -8,9 +8,7 @@ use {
rent_collector::RentCollector,
snapshot_archive_info::{SnapshotArchiveInfo, SnapshotArchiveInfoGetter},
snapshot_hash::SnapshotHash,
snapshot_utils::{
self, ArchiveFormat, BankSnapshotInfo, Result, SnapshotError, SnapshotVersion,
},
snapshot_utils::{self, ArchiveFormat, BankSnapshotInfo, Result, SnapshotVersion},
},
log::*,
solana_sdk::{clock::Slot, feature_set, sysvar::epoch_schedule::EpochSchedule},
@ -75,14 +73,8 @@ impl AccountsPackage {
}
}
let bank_snapshot_dir = &bank_snapshot_info.snapshot_dir;
let bank_snapshots_dir = bank_snapshot_dir
.parent()
.ok_or_else(|| SnapshotError::InvalidSnapshotDirPath(bank_snapshot_dir.to_path_buf()))?
.to_path_buf();
let snapshot_info = SupplementalSnapshotInfo {
snapshot_links: bank_snapshots_dir,
bank_snapshot_dir: bank_snapshot_info.snapshot_dir.clone(),
archive_format,
snapshot_version,
full_snapshot_archives_dir: full_snapshot_archives_dir.as_ref().to_path_buf(),
@ -159,7 +151,7 @@ impl AccountsPackage {
rent_collector: RentCollector::default(),
is_incremental_accounts_hash_feature_enabled: bool::default(),
snapshot_info: Some(SupplementalSnapshotInfo {
snapshot_links: PathBuf::default(),
bank_snapshot_dir: PathBuf::default(),
archive_format: ArchiveFormat::Tar,
snapshot_version: SnapshotVersion::default(),
full_snapshot_archives_dir: PathBuf::default(),
@ -170,16 +162,16 @@ impl AccountsPackage {
}
}
/// Returns the path to the snapshot links directory
/// Returns the path to the snapshot dir
///
/// NOTE: This fn will panic if the AccountsPackage is of type EpochAccountsHash.
pub fn snapshot_links_dir(&self) -> &Path {
pub fn bank_snapshot_dir(&self) -> &Path {
match self.package_type {
AccountsPackageType::AccountsHashVerifier | AccountsPackageType::Snapshot(..) => self
.snapshot_info
.as_ref()
.unwrap()
.snapshot_links
.bank_snapshot_dir
.as_path(),
AccountsPackageType::EpochAccountsHash => {
panic!("EAH accounts packages do not contain snapshot information")
@ -200,7 +192,7 @@ impl std::fmt::Debug for AccountsPackage {
/// Supplemental information needed for snapshots
pub struct SupplementalSnapshotInfo {
pub snapshot_links: PathBuf,
pub bank_snapshot_dir: PathBuf,
pub archive_format: ArchiveFormat,
pub snapshot_version: SnapshotVersion,
pub full_snapshot_archives_dir: PathBuf,
@ -222,7 +214,7 @@ pub enum AccountsPackageType {
pub struct SnapshotPackage {
pub snapshot_archive_info: SnapshotArchiveInfo,
pub block_height: Slot,
pub snapshot_links: PathBuf,
pub bank_snapshot_dir: PathBuf,
pub snapshot_storages: Vec<Arc<AccountStorageEntry>>,
pub snapshot_version: SnapshotVersion,
pub snapshot_type: SnapshotType,
@ -274,7 +266,7 @@ impl SnapshotPackage {
archive_format: snapshot_info.archive_format,
},
block_height: accounts_package.block_height,
snapshot_links: snapshot_info.snapshot_links,
bank_snapshot_dir: snapshot_info.bank_snapshot_dir,
snapshot_storages,
snapshot_version: snapshot_info.snapshot_version,
snapshot_type,

View File

@ -93,7 +93,7 @@ mod tests {
archive_format: ArchiveFormat::Tar,
},
block_height: slot,
snapshot_links: PathBuf::default(),
bank_snapshot_dir: PathBuf::default(),
snapshot_storages: Vec::default(),
snapshot_version: SnapshotVersion::default(),
snapshot_type,

View File

@ -647,7 +647,7 @@ pub fn archive_snapshot_package(
)
})?;
let src_snapshot_dir = snapshot_package.snapshot_links.join(&slot_str);
let src_snapshot_dir = &snapshot_package.bank_snapshot_dir;
// To be a source for symlinking and archiving, the path need to be an aboslute path
let src_snapshot_dir = src_snapshot_dir
.canonicalize()
@ -3132,7 +3132,7 @@ pub fn package_and_archive_full_snapshot(
.get_accounts_hash()
.expect("accounts hash is required for snapshot");
crate::serde_snapshot::reserialize_bank_with_new_accounts_hash(
accounts_package.snapshot_links_dir(),
accounts_package.bank_snapshot_dir(),
accounts_package.slot,
&accounts_hash,
None,
@ -3217,7 +3217,7 @@ pub fn package_and_archive_incremental_snapshot(
};
crate::serde_snapshot::reserialize_bank_with_new_accounts_hash(
accounts_package.snapshot_links_dir(),
accounts_package.bank_snapshot_dir(),
accounts_package.slot,
&accounts_hash_for_reserialize,
bank_incremental_snapshot_persistence.as_ref(),
@ -3282,7 +3282,7 @@ pub fn create_snapshot_dirs_for_tests(
let snapshot_storages = bank.get_snapshot_storages(None);
let slot_deltas = bank.status_cache.read().unwrap().root_slot_deltas();
add_bank_snapshot(
let bank_snapshot_info = add_bank_snapshot(
&bank_snapshots_dir,
&bank,
&snapshot_storages,
@ -3299,7 +3299,7 @@ pub fn create_snapshot_dirs_for_tests(
// to construct a bank.
assert!(
crate::serde_snapshot::reserialize_bank_with_new_accounts_hash(
&bank_snapshots_dir,
&bank_snapshot_info.snapshot_dir,
bank.slot(),
&bank.get_accounts_hash().unwrap(),
None