Skip to content

Commit

Permalink
Trims fields from snapshot package (#1438)
Browse files Browse the repository at this point in the history
  • Loading branch information
brooksprumo authored May 24, 2024
1 parent 529e73d commit a175ff1
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 60 deletions.
3 changes: 2 additions & 1 deletion core/src/accounts_hash_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 1 addition & 2 deletions core/src/snapshot_packager_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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(),
}
Expand Down
19 changes: 6 additions & 13 deletions core/tests/snapshots.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,7 @@ fn run_bank_forks_snapshot_n<F>(
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();
Expand All @@ -274,7 +270,8 @@ fn run_bank_forks_snapshot_n<F>(
&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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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());
Expand All @@ -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()
{
Expand Down
4 changes: 0 additions & 4 deletions runtime/src/accounts_background_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}
Expand Down
32 changes: 22 additions & 10 deletions runtime/src/snapshot_bank_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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,
);

Expand All @@ -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(
Expand All @@ -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,
);

Expand Down Expand Up @@ -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(
Expand Down
38 changes: 12 additions & 26 deletions runtime/src/snapshot_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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<PathBuf>,
incremental_snapshot_archives_dir: impl Into<PathBuf>,
snapshot_storages: Vec<Arc<AccountStorageEntry>>,
archive_format: ArchiveFormat,
snapshot_version: SnapshotVersion,
accounts_hash_for_testing: Option<AccountsHash>,
) -> Self {
if let AccountsPackageKind::Snapshot(snapshot_kind) = package_kind {
Expand All @@ -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(
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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<EpochAccountsHash>,
}

Expand All @@ -233,7 +217,6 @@ pub struct SnapshotPackage {
pub block_height: Slot,
pub bank_snapshot_dir: PathBuf,
pub snapshot_storages: Vec<Arc<AccountStorageEntry>>,
pub snapshot_version: SnapshotVersion,
pub snapshot_kind: SnapshotKind,

/// The instant this snapshot package was sent to the queue.
Expand All @@ -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!"
Expand All @@ -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);
Expand All @@ -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,
)
}
};
Expand All @@ -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(),
}
Expand Down
6 changes: 2 additions & 4 deletions runtime/src/snapshot_package/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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(),
}
Expand Down

0 comments on commit a175ff1

Please sign in to comment.