From a175ff1570e6e83d0dc9c5bf4bcb1ee89c0ced44 Mon Sep 17 00:00:00 2001 From: Brooks Date: Fri, 24 May 2024 19:30:19 -0400 Subject: [PATCH] Trims fields from snapshot package (#1438) --- core/src/accounts_hash_verifier.rs | 3 +- core/src/snapshot_packager_service.rs | 3 +- core/tests/snapshots.rs | 19 ++++------- runtime/src/accounts_background_service.rs | 4 --- runtime/src/snapshot_bank_utils.rs | 32 ++++++++++++------ runtime/src/snapshot_package.rs | 38 +++++++--------------- runtime/src/snapshot_package/compare.rs | 6 ++-- 7 files changed, 45 insertions(+), 60 deletions(-) diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs index 66a8475f699634..17e6e2fb388889 100644 --- a/core/src/accounts_hash_verifier.rs +++ b/core/src/accounts_hash_verifier.rs @@ -542,7 +542,8 @@ impl AccountsHashVerifier { return; }; - let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash); + let snapshot_package = + SnapshotPackage::new(accounts_package, accounts_hash, snapshot_config); let send_result = snapshot_package_sender.send(snapshot_package); if let Err(err) = send_result { // Sending the snapshot package should never fail *unless* we're shutting down. diff --git a/core/src/snapshot_packager_service.rs b/core/src/snapshot_packager_service.rs index c8907951e17d5c..6dd328bb60feb2 100644 --- a/core/src/snapshot_packager_service.rs +++ b/core/src/snapshot_packager_service.rs @@ -206,7 +206,7 @@ mod tests { snapshot_archive_info::SnapshotArchiveInfo, snapshot_hash::SnapshotHash, snapshot_package::{SnapshotKind, SnapshotPackage}, - snapshot_utils::{ArchiveFormat, SnapshotVersion}, + snapshot_utils::ArchiveFormat, }, solana_sdk::{clock::Slot, hash::Hash}, std::{path::PathBuf, time::Instant}, @@ -230,7 +230,6 @@ mod tests { block_height: slot, bank_snapshot_dir: PathBuf::default(), snapshot_storages: Vec::default(), - snapshot_version: SnapshotVersion::default(), snapshot_kind, enqueued: Instant::now(), } diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 2ff6f85a08ab5c..1df853879e09ab 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -258,11 +258,7 @@ fn run_bank_forks_snapshot_n( AccountsPackageKind::Snapshot(SnapshotKind::FullSnapshot), &last_bank, &last_bank_snapshot_info, - &snapshot_config.full_snapshot_archives_dir, - &snapshot_config.incremental_snapshot_archives_dir, last_bank.get_snapshot_storages(None), - snapshot_config.archive_format, - snapshot_version, None, ); last_bank.force_flush_accounts_cache(); @@ -274,7 +270,8 @@ fn run_bank_forks_snapshot_n( &accounts_hash, None, ); - let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash.into()); + let snapshot_package = + SnapshotPackage::new(accounts_package, accounts_hash.into(), snapshot_config); snapshot_utils::archive_snapshot_package(&snapshot_package).unwrap(); // Restore bank from snapshot @@ -351,7 +348,6 @@ fn test_concurrent_snapshot_packaging( let snapshot_config = &snapshot_test_config.snapshot_config; let bank_snapshots_dir = &snapshot_config.bank_snapshots_dir; let full_snapshot_archives_dir = &snapshot_config.full_snapshot_archives_dir; - let incremental_snapshot_archives_dir = &snapshot_config.incremental_snapshot_archives_dir; let mint_keypair = &snapshot_test_config.genesis_config_info.mint_keypair; let genesis_config = &snapshot_test_config.genesis_config_info.genesis_config; @@ -429,11 +425,7 @@ fn test_concurrent_snapshot_packaging( AccountsPackageKind::Snapshot(SnapshotKind::FullSnapshot), &bank, &bank_snapshot_info, - full_snapshot_archives_dir, - incremental_snapshot_archives_dir, snapshot_storages, - snapshot_config.archive_format, - snapshot_config.snapshot_version, None, ); accounts_package_sender.send(accounts_package).unwrap(); @@ -539,7 +531,7 @@ fn test_concurrent_snapshot_packaging( let _package_receiver = std::thread::Builder::new() .name("package-receiver".to_string()) .spawn({ - let full_snapshot_archives_dir = full_snapshot_archives_dir.clone(); + let snapshot_config = snapshot_config.clone(); move || { let accounts_package = real_accounts_package_receiver.try_recv().unwrap(); let accounts_hash = AccountsHash(Hash::default()); @@ -549,12 +541,13 @@ fn test_concurrent_snapshot_packaging( &accounts_hash, None, ); - let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash.into()); + let snapshot_package = + SnapshotPackage::new(accounts_package, accounts_hash.into(), &snapshot_config); snapshot_package_sender.send(snapshot_package).unwrap(); // Wait until the package has been archived by SnapshotPackagerService while snapshot_utils::get_highest_full_snapshot_archive_slot( - &full_snapshot_archives_dir, + &snapshot_config.full_snapshot_archives_dir, ) .is_none() { diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index 6b76e0f7fefaa5..d49294d2941074 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -394,11 +394,7 @@ impl SnapshotRequestHandler { accounts_package_kind, &snapshot_root_bank, &bank_snapshot_info, - &self.snapshot_config.full_snapshot_archives_dir, - &self.snapshot_config.incremental_snapshot_archives_dir, snapshot_storages, - self.snapshot_config.archive_format, - self.snapshot_config.snapshot_version, accounts_hash_for_testing, ) } diff --git a/runtime/src/snapshot_bank_utils.rs b/runtime/src/snapshot_bank_utils.rs index 6342450418eff4..1e354e16bf767e 100644 --- a/runtime/src/snapshot_bank_utils.rs +++ b/runtime/src/snapshot_bank_utils.rs @@ -12,6 +12,7 @@ use { snapshot_archive_info::{ FullSnapshotArchiveInfo, IncrementalSnapshotArchiveInfo, SnapshotArchiveInfoGetter, }, + snapshot_config::SnapshotConfig, snapshot_hash::SnapshotHash, snapshot_package::{AccountsPackage, AccountsPackageKind, SnapshotKind, SnapshotPackage}, snapshot_utils::{ @@ -1123,11 +1124,7 @@ fn package_and_archive_full_snapshot( AccountsPackageKind::Snapshot(SnapshotKind::FullSnapshot), bank, bank_snapshot_info, - full_snapshot_archives_dir.as_ref(), - incremental_snapshot_archives_dir.as_ref(), snapshot_storages, - archive_format, - snapshot_version, None, ); @@ -1141,7 +1138,15 @@ fn package_and_archive_full_snapshot( None, ); - let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash.into()); + let snapshot_config = SnapshotConfig { + 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(), + archive_format, + snapshot_version, + ..Default::default() + }; + let snapshot_package = + SnapshotPackage::new(accounts_package, accounts_hash.into(), &snapshot_config); archive_snapshot_package(&snapshot_package)?; Ok(FullSnapshotArchiveInfo::new( @@ -1168,11 +1173,7 @@ fn package_and_archive_incremental_snapshot( )), bank, bank_snapshot_info, - full_snapshot_archives_dir.as_ref(), - incremental_snapshot_archives_dir.as_ref(), snapshot_storages, - archive_format, - snapshot_version, None, ); @@ -1202,7 +1203,18 @@ fn package_and_archive_incremental_snapshot( bank_incremental_snapshot_persistence.as_ref(), ); - let snapshot_package = SnapshotPackage::new(accounts_package, incremental_accounts_hash.into()); + let snapshot_config = SnapshotConfig { + 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(), + archive_format, + snapshot_version, + ..Default::default() + }; + let snapshot_package = SnapshotPackage::new( + accounts_package, + incremental_accounts_hash.into(), + &snapshot_config, + ); archive_snapshot_package(&snapshot_package)?; Ok(IncrementalSnapshotArchiveInfo::new( diff --git a/runtime/src/snapshot_package.rs b/runtime/src/snapshot_package.rs index 5f8b00060e9d58..abef93bfebaf34 100644 --- a/runtime/src/snapshot_package.rs +++ b/runtime/src/snapshot_package.rs @@ -2,8 +2,9 @@ use { crate::{ bank::Bank, snapshot_archive_info::{SnapshotArchiveInfo, SnapshotArchiveInfoGetter}, + snapshot_config::SnapshotConfig, snapshot_hash::SnapshotHash, - snapshot_utils::{self, ArchiveFormat, BankSnapshotInfo, SnapshotVersion}, + snapshot_utils::{self, BankSnapshotInfo}, }, log::*, solana_accounts_db::{ @@ -47,16 +48,11 @@ pub struct AccountsPackage { impl AccountsPackage { /// Package up bank files, storages, and slot deltas for a snapshot - #[allow(clippy::too_many_arguments)] pub fn new_for_snapshot( package_kind: AccountsPackageKind, bank: &Bank, bank_snapshot_info: &BankSnapshotInfo, - full_snapshot_archives_dir: impl Into, - incremental_snapshot_archives_dir: impl Into, snapshot_storages: Vec>, - archive_format: ArchiveFormat, - snapshot_version: SnapshotVersion, accounts_hash_for_testing: Option, ) -> Self { if let AccountsPackageKind::Snapshot(snapshot_kind) = package_kind { @@ -77,10 +73,6 @@ impl AccountsPackage { let snapshot_info = SupplementalSnapshotInfo { bank_snapshot_dir: bank_snapshot_info.snapshot_dir.clone(), - archive_format, - snapshot_version, - full_snapshot_archives_dir: full_snapshot_archives_dir.into(), - incremental_snapshot_archives_dir: incremental_snapshot_archives_dir.into(), epoch_accounts_hash: bank.get_epoch_accounts_hash_to_serialize(), }; Self::_new( @@ -169,10 +161,6 @@ impl AccountsPackage { rent_collector: RentCollector::default(), snapshot_info: Some(SupplementalSnapshotInfo { bank_snapshot_dir: PathBuf::default(), - archive_format: ArchiveFormat::Tar, - snapshot_version: SnapshotVersion::default(), - full_snapshot_archives_dir: PathBuf::default(), - incremental_snapshot_archives_dir: PathBuf::default(), epoch_accounts_hash: Option::default(), }), enqueued: Instant::now(), @@ -210,10 +198,6 @@ impl std::fmt::Debug for AccountsPackage { /// Supplemental information needed for snapshots pub struct SupplementalSnapshotInfo { pub bank_snapshot_dir: PathBuf, - pub archive_format: ArchiveFormat, - pub snapshot_version: SnapshotVersion, - pub full_snapshot_archives_dir: PathBuf, - pub incremental_snapshot_archives_dir: PathBuf, pub epoch_accounts_hash: Option, } @@ -233,7 +217,6 @@ pub struct SnapshotPackage { pub block_height: Slot, pub bank_snapshot_dir: PathBuf, pub snapshot_storages: Vec>, - pub snapshot_version: SnapshotVersion, pub snapshot_kind: SnapshotKind, /// The instant this snapshot package was sent to the queue. @@ -242,7 +225,11 @@ pub struct SnapshotPackage { } impl SnapshotPackage { - pub fn new(accounts_package: AccountsPackage, accounts_hash: AccountsHashKind) -> Self { + pub fn new( + accounts_package: AccountsPackage, + accounts_hash: AccountsHashKind, + snapshot_config: &SnapshotConfig, + ) -> Self { let AccountsPackageKind::Snapshot(snapshot_kind) = accounts_package.package_kind else { panic!( "The AccountsPackage must be of kind Snapshot in order to make a SnapshotPackage!" @@ -258,10 +245,10 @@ impl SnapshotPackage { let mut snapshot_storages = accounts_package.snapshot_storages; let snapshot_archive_path = match snapshot_kind { SnapshotKind::FullSnapshot => snapshot_utils::build_full_snapshot_archive_path( - snapshot_info.full_snapshot_archives_dir, + &snapshot_config.full_snapshot_archives_dir, accounts_package.slot, &snapshot_hash, - snapshot_info.archive_format, + snapshot_config.archive_format, ), SnapshotKind::IncrementalSnapshot(incremental_snapshot_base_slot) => { snapshot_storages.retain(|storage| storage.slot() > incremental_snapshot_base_slot); @@ -270,11 +257,11 @@ impl SnapshotPackage { "Incremental snapshot package must only contain storage entries where slot > incremental snapshot base slot (i.e. full snapshot slot)!" ); snapshot_utils::build_incremental_snapshot_archive_path( - snapshot_info.incremental_snapshot_archives_dir, + &snapshot_config.incremental_snapshot_archives_dir, incremental_snapshot_base_slot, accounts_package.slot, &snapshot_hash, - snapshot_info.archive_format, + snapshot_config.archive_format, ) } }; @@ -284,12 +271,11 @@ impl SnapshotPackage { path: snapshot_archive_path, slot: accounts_package.slot, hash: snapshot_hash, - archive_format: snapshot_info.archive_format, + archive_format: snapshot_config.archive_format, }, block_height: accounts_package.block_height, bank_snapshot_dir: snapshot_info.bank_snapshot_dir, snapshot_storages, - snapshot_version: snapshot_info.snapshot_version, snapshot_kind, enqueued: Instant::now(), } diff --git a/runtime/src/snapshot_package/compare.rs b/runtime/src/snapshot_package/compare.rs index d951d818c37975..dad31d8fc1852b 100644 --- a/runtime/src/snapshot_package/compare.rs +++ b/runtime/src/snapshot_package/compare.rs @@ -74,9 +74,8 @@ mod tests { use { super::*, crate::{ - snapshot_archive_info::SnapshotArchiveInfo, - snapshot_hash::SnapshotHash, - snapshot_utils::{ArchiveFormat, SnapshotVersion}, + snapshot_archive_info::SnapshotArchiveInfo, snapshot_hash::SnapshotHash, + snapshot_utils::ArchiveFormat, }, solana_sdk::{clock::Slot, hash::Hash}, std::{path::PathBuf, time::Instant}, @@ -95,7 +94,6 @@ mod tests { block_height: slot, bank_snapshot_dir: PathBuf::default(), snapshot_storages: Vec::default(), - snapshot_version: SnapshotVersion::default(), snapshot_kind, enqueued: Instant::now(), }