diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs index 14c2dd8700..6f60e0ba71 100644 --- a/core/src/accounts_hash_verifier.rs +++ b/core/src/accounts_hash_verifier.rs @@ -298,12 +298,14 @@ impl AccountsHashVerifier { ); measure_hash.stop(); - solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash( - accounts_package.snapshot_links.path(), - accounts_package.slot, - &accounts_hash, - None, - ); + if let Some(snapshot_info) = &accounts_package.snapshot_info { + solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash( + snapshot_info.snapshot_links.path(), + accounts_package.slot, + &accounts_hash, + None, + ); + } 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 a052578b0f..158f1e08cf 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -262,7 +262,7 @@ fn run_bank_forks_snapshot_n( let last_bank_snapshot_info = snapshot_utils::get_highest_bank_snapshot_pre(bank_snapshots_dir) .expect("no bank snapshots found in path"); let slot_deltas = last_bank.status_cache.read().unwrap().root_slot_deltas(); - let accounts_package = AccountsPackage::new( + let accounts_package = AccountsPackage::new_for_snapshot( AccountsPackageType::Snapshot(SnapshotType::FullSnapshot), &last_bank, &last_bank_snapshot_info, @@ -277,7 +277,7 @@ fn run_bank_forks_snapshot_n( ) .unwrap(); solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash( - accounts_package.snapshot_links.path(), + accounts_package.snapshot_links_dir(), accounts_package.slot, &last_bank.get_accounts_hash(), None, @@ -416,7 +416,7 @@ fn test_concurrent_snapshot_packaging( snapshot_config.snapshot_version, ) .unwrap(); - let accounts_package = AccountsPackage::new( + let accounts_package = AccountsPackage::new_for_snapshot( AccountsPackageType::Snapshot(SnapshotType::FullSnapshot), &bank, &bank_snapshot_info, @@ -524,7 +524,7 @@ fn test_concurrent_snapshot_packaging( .spawn(move || { let accounts_package = real_accounts_package_receiver.try_recv().unwrap(); solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash( - accounts_package.snapshot_links.path(), + accounts_package.snapshot_links_dir(), accounts_package.slot, &Hash::default(), None, diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index 750e1f543a..5f4b407595 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -382,7 +382,7 @@ impl SnapshotRequestHandler { self.snapshot_config.snapshot_version, ) .expect("snapshot bank"); - let accounts_package = AccountsPackage::new( + let accounts_package = AccountsPackage::new_for_snapshot( accounts_package_type, &snapshot_root_bank, &bank_snapshot_info, diff --git a/runtime/src/snapshot_package.rs b/runtime/src/snapshot_package.rs index fef96cd90d..2c9e1052cf 100644 --- a/runtime/src/snapshot_package.rs +++ b/runtime/src/snapshot_package.rs @@ -29,23 +29,21 @@ pub use compare::*; /// SnapshotPackagerService for archiving pub type PendingSnapshotPackage = Arc>>; +/// This struct packages up fields to send from AccountsBackgroundService to AccountsHashVerifier pub struct AccountsPackage { pub package_type: AccountsPackageType, pub slot: Slot, pub block_height: Slot, - pub slot_deltas: Vec, - pub snapshot_links: TempDir, pub snapshot_storages: SnapshotStorages, - pub archive_format: ArchiveFormat, - pub snapshot_version: SnapshotVersion, - pub full_snapshot_archives_dir: PathBuf, - pub incremental_snapshot_archives_dir: PathBuf, pub expected_capitalization: u64, pub accounts_hash_for_testing: Option, pub accounts: Arc, pub epoch_schedule: EpochSchedule, pub rent_collector: RentCollector, + /// Supplemental information needed for snapshots + pub snapshot_info: Option, + /// The instant this accounts package was send to the queue. /// Used to track how long accounts packages wait before processing. pub enqueued: Instant, @@ -54,7 +52,7 @@ pub struct AccountsPackage { impl AccountsPackage { /// Package up bank files, storages, and slot deltas for a snapshot #[allow(clippy::too_many_arguments)] - pub fn new( + pub fn new_for_snapshot( package_type: AccountsPackageType, bank: &Bank, bank_snapshot_info: &BankSnapshotInfo, @@ -100,26 +98,45 @@ impl AccountsPackage { )?; } - Ok(Self { - package_type, - slot: bank.slot(), - block_height: bank.block_height(), + let snapshot_info = SupplementalSnapshotInfo { slot_deltas, snapshot_links, - snapshot_storages, archive_format, snapshot_version, full_snapshot_archives_dir: full_snapshot_archives_dir.as_ref().to_path_buf(), incremental_snapshot_archives_dir: incremental_snapshot_archives_dir .as_ref() .to_path_buf(), + }; + Ok(Self::_new( + package_type, + bank, + snapshot_storages, + accounts_hash_for_testing, + Some(snapshot_info), + )) + } + + fn _new( + package_type: AccountsPackageType, + bank: &Bank, + snapshot_storages: SnapshotStorages, + accounts_hash_for_testing: Option, + snapshot_info: Option, + ) -> Self { + Self { + package_type, + slot: bank.slot(), + block_height: bank.block_height(), + snapshot_storages, expected_capitalization: bank.capitalization(), accounts_hash_for_testing, accounts: bank.accounts(), epoch_schedule: *bank.epoch_schedule(), rent_collector: bank.rent_collector().clone(), + snapshot_info, enqueued: Instant::now(), - }) + } } /// Create a new Accounts Package where basically every field is defaulted. @@ -129,21 +146,39 @@ impl AccountsPackage { package_type: AccountsPackageType::AccountsHashVerifier, slot: Slot::default(), block_height: Slot::default(), - slot_deltas: Vec::default(), - snapshot_links: TempDir::new().unwrap(), snapshot_storages: SnapshotStorages::default(), - archive_format: ArchiveFormat::Tar, - snapshot_version: SnapshotVersion::default(), - full_snapshot_archives_dir: PathBuf::default(), - incremental_snapshot_archives_dir: PathBuf::default(), expected_capitalization: u64::default(), accounts_hash_for_testing: Option::default(), accounts: Arc::new(Accounts::default_for_tests()), epoch_schedule: EpochSchedule::default(), rent_collector: RentCollector::default(), + snapshot_info: Some(SupplementalSnapshotInfo { + slot_deltas: Vec::default(), + snapshot_links: TempDir::new().unwrap(), + archive_format: ArchiveFormat::Tar, + snapshot_version: SnapshotVersion::default(), + full_snapshot_archives_dir: PathBuf::default(), + incremental_snapshot_archives_dir: PathBuf::default(), + }), enqueued: Instant::now(), } } + + /// Returns the path to the snapshot links directory + /// + /// NOTE 1: This path is within the TempDir created for the AccountsPackage, *not* the bank + /// snapshots dir passed into `new_for_snapshot()` when creating the AccountsPackage. + /// NOTE 2: This fn will panic if the AccountsPackage is of type EpochAccountsHash. + pub fn snapshot_links_dir(&self) -> &Path { + match self.package_type { + AccountsPackageType::AccountsHashVerifier | AccountsPackageType::Snapshot(..) => { + self.snapshot_info.as_ref().unwrap().snapshot_links.path() + } + AccountsPackageType::EpochAccountsHash => { + panic!("EAH accounts packages do not contain snapshot information") + } + } + } } impl std::fmt::Debug for AccountsPackage { @@ -156,6 +191,16 @@ impl std::fmt::Debug for AccountsPackage { } } +/// Supplemental information needed for snapshots +pub struct SupplementalSnapshotInfo { + pub slot_deltas: Vec, + pub snapshot_links: TempDir, + pub archive_format: ArchiveFormat, + pub snapshot_version: SnapshotVersion, + pub full_snapshot_archives_dir: PathBuf, + pub incremental_snapshot_archives_dir: PathBuf, +} + /// Accounts packages are sent to the Accounts Hash Verifier for processing. There are multiple /// types of accounts packages, which are specified as variants in this enum. All accounts /// packages do share some processing: such as calculating the accounts hash. @@ -178,6 +223,11 @@ pub struct SnapshotPackage { impl SnapshotPackage { pub fn new(accounts_package: AccountsPackage, accounts_hash: Hash) -> Self { + assert!( + accounts_package.snapshot_info.is_some(), + "The AccountsPackage must have snapshot info in order to make a SnapshotPackage!" + ); + let snapshot_info = accounts_package.snapshot_info.unwrap(); let snapshot_hash = SnapshotHash::new(&accounts_hash); let mut snapshot_storages = accounts_package.snapshot_storages; let (snapshot_type, snapshot_archive_path) = match accounts_package.package_type { @@ -185,10 +235,10 @@ impl SnapshotPackage { SnapshotType::FullSnapshot => ( snapshot_type, snapshot_utils::build_full_snapshot_archive_path( - accounts_package.full_snapshot_archives_dir, + snapshot_info.full_snapshot_archives_dir, accounts_package.slot, &snapshot_hash, - accounts_package.archive_format, + snapshot_info.archive_format, ), ), SnapshotType::IncrementalSnapshot(incremental_snapshot_base_slot) => { @@ -207,11 +257,11 @@ impl SnapshotPackage { ( snapshot_type, snapshot_utils::build_incremental_snapshot_archive_path( - accounts_package.incremental_snapshot_archives_dir, + snapshot_info.incremental_snapshot_archives_dir, incremental_snapshot_base_slot, accounts_package.slot, &snapshot_hash, - accounts_package.archive_format, + snapshot_info.archive_format, ), ) } @@ -226,13 +276,13 @@ impl SnapshotPackage { path: snapshot_archive_path, slot: accounts_package.slot, hash: snapshot_hash, - archive_format: accounts_package.archive_format, + archive_format: snapshot_info.archive_format, }, block_height: accounts_package.block_height, - slot_deltas: accounts_package.slot_deltas, - snapshot_links: accounts_package.snapshot_links, + slot_deltas: snapshot_info.slot_deltas, + snapshot_links: snapshot_info.snapshot_links, snapshot_storages, - snapshot_version: accounts_package.snapshot_version, + snapshot_version: snapshot_info.snapshot_version, snapshot_type, } } diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 8c67f94528..eaf790d770 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -2210,7 +2210,7 @@ pub fn package_and_archive_full_snapshot( maximum_incremental_snapshot_archives_to_retain: usize, ) -> Result { let slot_deltas = bank.status_cache.read().unwrap().root_slot_deltas(); - let accounts_package = AccountsPackage::new( + let accounts_package = AccountsPackage::new_for_snapshot( AccountsPackageType::Snapshot(SnapshotType::FullSnapshot), bank, bank_snapshot_info, @@ -2225,7 +2225,7 @@ pub fn package_and_archive_full_snapshot( )?; crate::serde_snapshot::reserialize_bank_with_new_accounts_hash( - accounts_package.snapshot_links.path(), + accounts_package.snapshot_links_dir(), accounts_package.slot, &bank.get_accounts_hash(), None, @@ -2261,7 +2261,7 @@ pub fn package_and_archive_incremental_snapshot( maximum_incremental_snapshot_archives_to_retain: usize, ) -> Result { let slot_deltas = bank.status_cache.read().unwrap().root_slot_deltas(); - let accounts_package = AccountsPackage::new( + let accounts_package = AccountsPackage::new_for_snapshot( AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot( incremental_snapshot_base_slot, )), @@ -2278,7 +2278,7 @@ pub fn package_and_archive_incremental_snapshot( )?; crate::serde_snapshot::reserialize_bank_with_new_accounts_hash( - accounts_package.snapshot_links.path(), + accounts_package.snapshot_links_dir(), accounts_package.slot, &bank.get_accounts_hash(), None,