Make Accounts Background Services aware of Epoch Accounts Hash (#27626)

This commit is contained in:
Brooks Prumo 2022-09-07 16:41:40 -04:00 committed by GitHub
parent 7f223dc582
commit 6a322de845
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 326 additions and 186 deletions

View File

@ -9,10 +9,11 @@ use {
solana_measure::measure::Measure, solana_measure::measure::Measure,
solana_runtime::{ solana_runtime::{
accounts_hash::{CalcAccountsHashConfig, HashStats}, accounts_hash::{CalcAccountsHashConfig, HashStats},
epoch_accounts_hash::EpochAccountsHash,
snapshot_config::SnapshotConfig, snapshot_config::SnapshotConfig,
snapshot_package::{ snapshot_package::{
retain_max_n_elements, AccountsPackage, PendingAccountsPackage, PendingSnapshotPackage, retain_max_n_elements, AccountsPackage, AccountsPackageType, PendingAccountsPackage,
SnapshotPackage, SnapshotType, PendingSnapshotPackage, SnapshotPackage, SnapshotType,
}, },
sorted_storages::SortedStorages, sorted_storages::SortedStorages,
}, },
@ -98,6 +99,8 @@ impl AccountsHashVerifier {
) { ) {
let accounts_hash = Self::calculate_and_verify_accounts_hash(&accounts_package); let accounts_hash = Self::calculate_and_verify_accounts_hash(&accounts_package);
Self::save_epoch_accounts_hash(&accounts_package, accounts_hash);
Self::push_accounts_hashes_to_cluster( Self::push_accounts_hashes_to_cluster(
&accounts_package, &accounts_package,
cluster_info, cluster_info,
@ -228,6 +231,21 @@ impl AccountsHashVerifier {
accounts_hash accounts_hash
} }
fn save_epoch_accounts_hash(accounts_package: &AccountsPackage, accounts_hash: Hash) {
if accounts_package.package_type == AccountsPackageType::EpochAccountsHash {
let new_epoch_accounts_hash = EpochAccountsHash::new(accounts_hash);
let old_epoch_accounts_hash = accounts_package
.accounts
.accounts_db
.epoch_accounts_hash
.lock()
.unwrap()
.replace(new_epoch_accounts_hash);
// Old epoch accounts hash must be NONE, because a previous bank must have taken it to hash into itself
assert!(old_epoch_accounts_hash.is_none());
}
}
fn generate_fault_hash(original_hash: &Hash) -> Hash { fn generate_fault_hash(original_hash: &Hash) -> Hash {
use { use {
rand::{thread_rng, Rng}, rand::{thread_rng, Rng},
@ -280,14 +298,17 @@ impl AccountsHashVerifier {
snapshot_config: Option<&SnapshotConfig>, snapshot_config: Option<&SnapshotConfig>,
accounts_hash: Hash, accounts_hash: Hash,
) { ) {
if accounts_package.snapshot_type.is_none() if pending_snapshot_package.is_none()
|| pending_snapshot_package.is_none()
|| !snapshot_config || !snapshot_config
.map(|snapshot_config| snapshot_config.should_generate_snapshots()) .map(|snapshot_config| snapshot_config.should_generate_snapshots())
.unwrap_or(false) .unwrap_or(false)
|| !matches!(
accounts_package.package_type,
AccountsPackageType::Snapshot(_)
)
{ {
return; return;
}; }
let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash); let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash);
let pending_snapshot_package = pending_snapshot_package.unwrap(); let pending_snapshot_package = pending_snapshot_package.unwrap();
@ -444,6 +465,7 @@ mod tests {
let expected_hash = Hash::from_str("GKot5hBsd81kMupNCXHaqbhv3huEbxAFMLnpcX2hniwn").unwrap(); let expected_hash = Hash::from_str("GKot5hBsd81kMupNCXHaqbhv3huEbxAFMLnpcX2hniwn").unwrap();
for i in 0..MAX_SNAPSHOT_HASHES + 1 { for i in 0..MAX_SNAPSHOT_HASHES + 1 {
let accounts_package = AccountsPackage { let accounts_package = AccountsPackage {
package_type: AccountsPackageType::AccountsHashVerifier,
slot: full_snapshot_archive_interval_slots + i as u64, slot: full_snapshot_archive_interval_slots + i as u64,
block_height: full_snapshot_archive_interval_slots + i as u64, block_height: full_snapshot_archive_interval_slots + i as u64,
slot_deltas: vec![], slot_deltas: vec![],
@ -456,7 +478,6 @@ mod tests {
expected_capitalization: 0, expected_capitalization: 0,
accounts_hash_for_testing: None, accounts_hash_for_testing: None,
cluster_type: ClusterType::MainnetBeta, cluster_type: ClusterType::MainnetBeta,
snapshot_type: None,
accounts: Arc::clone(&accounts), accounts: Arc::clone(&accounts),
epoch_schedule: EpochSchedule::default(), epoch_schedule: EpochSchedule::default(),
rent_collector: RentCollector::default(), rent_collector: RentCollector::default(),

View File

@ -25,8 +25,8 @@ use {
snapshot_archive_info::FullSnapshotArchiveInfo, snapshot_archive_info::FullSnapshotArchiveInfo,
snapshot_config::SnapshotConfig, snapshot_config::SnapshotConfig,
snapshot_package::{ snapshot_package::{
AccountsPackage, PendingAccountsPackage, PendingSnapshotPackage, SnapshotPackage, AccountsPackage, AccountsPackageType, PendingAccountsPackage, PendingSnapshotPackage,
SnapshotType, SnapshotPackage, SnapshotType,
}, },
snapshot_utils::{ snapshot_utils::{
self, ArchiveFormat, self, ArchiveFormat,
@ -240,6 +240,7 @@ fn run_bank_forks_snapshot_n<F>(
.expect("no bank snapshots found in path"); .expect("no bank snapshots found in path");
let slot_deltas = last_bank.status_cache.read().unwrap().root_slot_deltas(); let slot_deltas = last_bank.status_cache.read().unwrap().root_slot_deltas();
let accounts_package = AccountsPackage::new( let accounts_package = AccountsPackage::new(
AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
&last_bank, &last_bank,
&last_bank_snapshot_info, &last_bank_snapshot_info,
bank_snapshots_dir, bank_snapshots_dir,
@ -250,7 +251,6 @@ fn run_bank_forks_snapshot_n<F>(
ArchiveFormat::TarBzip2, ArchiveFormat::TarBzip2,
snapshot_version, snapshot_version,
None, None,
Some(SnapshotType::FullSnapshot),
) )
.unwrap(); .unwrap();
solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash( solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash(
@ -391,7 +391,7 @@ fn test_concurrent_snapshot_packaging(
snapshot_config.snapshot_version, snapshot_config.snapshot_version,
snapshot_config.archive_format, snapshot_config.archive_format,
None, None,
Some(SnapshotType::FullSnapshot), AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
) )
.unwrap(); .unwrap();

View File

@ -9,7 +9,7 @@ use {
bank::{Bank, BankSlotDelta, DropCallback}, bank::{Bank, BankSlotDelta, DropCallback},
bank_forks::BankForks, bank_forks::BankForks,
snapshot_config::SnapshotConfig, snapshot_config::SnapshotConfig,
snapshot_package::{PendingAccountsPackage, SnapshotType}, snapshot_package::{AccountsPackageType, PendingAccountsPackage, SnapshotType},
snapshot_utils::{self, SnapshotError}, snapshot_utils::{self, SnapshotError},
}, },
crossbeam_channel::{Receiver, SendError, Sender}, crossbeam_channel::{Receiver, SendError, Sender},
@ -112,6 +112,18 @@ impl SendDroppedBankCallback {
pub struct SnapshotRequest { pub struct SnapshotRequest {
pub snapshot_root_bank: Arc<Bank>, pub snapshot_root_bank: Arc<Bank>,
pub status_cache_slot_deltas: Vec<BankSlotDelta>, pub status_cache_slot_deltas: Vec<BankSlotDelta>,
pub request_type: SnapshotRequestType,
}
/// What type of request is this?
///
/// The snapshot request has been expanded to support more than just snapshots. This is
/// confusing, but can be resolved by renaming this type; or better, by creating an enum with
/// variants that wrap the fields-of-interest for each request.
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub enum SnapshotRequestType {
Snapshot,
EpochAccountsHash,
} }
pub struct SnapshotRequestHandler { pub struct SnapshotRequestHandler {
@ -137,6 +149,7 @@ impl SnapshotRequestHandler {
let SnapshotRequest { let SnapshotRequest {
snapshot_root_bank, snapshot_root_bank,
status_cache_slot_deltas, status_cache_slot_deltas,
request_type,
} = snapshot_request; } = snapshot_request;
// we should not rely on the state of this validator until startup verification is complete // we should not rely on the state of this validator until startup verification is complete
@ -216,23 +229,19 @@ impl SnapshotRequestHandler {
} }
let block_height = snapshot_root_bank.block_height(); let block_height = snapshot_root_bank.block_height();
let snapshot_type = if snapshot_utils::should_take_full_snapshot( let accounts_package_type = match request_type {
block_height, SnapshotRequestType::EpochAccountsHash => AccountsPackageType::EpochAccountsHash,
self.snapshot_config.full_snapshot_archive_interval_slots, _ => {
) { if snapshot_utils::should_take_full_snapshot(block_height, self.snapshot_config.full_snapshot_archive_interval_slots) {
*last_full_snapshot_slot = Some(snapshot_root_bank.slot()); *last_full_snapshot_slot = Some(snapshot_root_bank.slot());
Some(SnapshotType::FullSnapshot) AccountsPackageType::Snapshot(SnapshotType::FullSnapshot)
} else if snapshot_utils::should_take_incremental_snapshot( } else if snapshot_utils::should_take_incremental_snapshot(block_height, self.snapshot_config .incremental_snapshot_archive_interval_slots, *last_full_snapshot_slot) {
block_height, AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot( last_full_snapshot_slot.unwrap()))
self.snapshot_config } else {
.incremental_snapshot_archive_interval_slots, AccountsPackageType::AccountsHashVerifier
*last_full_snapshot_slot, }
) { },
Some(SnapshotType::IncrementalSnapshot(
last_full_snapshot_slot.unwrap(),
))
} else {
None
}; };
// Snapshot the bank and send over an accounts package // Snapshot the bank and send over an accounts package
@ -247,13 +256,13 @@ impl SnapshotRequestHandler {
self.snapshot_config.snapshot_version, self.snapshot_config.snapshot_version,
self.snapshot_config.archive_format, self.snapshot_config.archive_format,
hash_for_testing, hash_for_testing,
snapshot_type, accounts_package_type,
); );
if let Err(e) = result { if let Err(e) = result {
warn!( warn!(
"Error taking bank snapshot. slot: {}, snapshot type: {:?}, err: {:?}", "Error taking bank snapshot. slot: {}, accounts package type: {:?}, err: {:?}",
snapshot_root_bank.slot(), snapshot_root_bank.slot(),
snapshot_type, accounts_package_type,
e, e,
); );
@ -262,8 +271,8 @@ impl SnapshotRequestHandler {
} }
} }
snapshot_time.stop(); snapshot_time.stop();
info!("Took bank snapshot. snapshot type: {:?}, slot: {}, accounts hash: {}, bank hash: {}", info!("Took bank snapshot. accounts package type: {:?}, slot: {}, accounts hash: {}, bank hash: {}",
snapshot_type, accounts_package_type,
snapshot_root_bank.slot(), snapshot_root_bank.slot(),
snapshot_root_bank.get_accounts_hash(), snapshot_root_bank.get_accounts_hash(),
snapshot_root_bank.hash(), snapshot_root_bank.hash(),

View File

@ -1195,7 +1195,7 @@ pub struct AccountsDb {
/// The cadence is once per epoch, all nodes calculate a full accounts hash as of a known slot calculated using 'N' /// The cadence is once per epoch, all nodes calculate a full accounts hash as of a known slot calculated using 'N'
/// Some time later (to allow for slow calculation time), the bank hash at a slot calculated using 'M' includes the full accounts hash. /// Some time later (to allow for slow calculation time), the bank hash at a slot calculated using 'M' includes the full accounts hash.
/// Thus, the state of all accounts on a validator is known to be correct at least once per epoch. /// Thus, the state of all accounts on a validator is known to be correct at least once per epoch.
pub(crate) epoch_accounts_hash: Mutex<Option<EpochAccountsHash>>, pub epoch_accounts_hash: Mutex<Option<EpochAccountsHash>>,
} }
#[derive(Debug, Default)] #[derive(Debug, Default)]

View File

@ -2,7 +2,7 @@
use { use {
crate::{ crate::{
accounts_background_service::{AbsRequestSender, SnapshotRequest}, accounts_background_service::{AbsRequestSender, SnapshotRequest, SnapshotRequestType},
bank::Bank, bank::Bank,
snapshot_config::SnapshotConfig, snapshot_config::SnapshotConfig,
}, },
@ -290,6 +290,7 @@ impl BankForks {
SnapshotRequest { SnapshotRequest {
snapshot_root_bank, snapshot_root_bank,
status_cache_slot_deltas, status_cache_slot_deltas,
request_type: SnapshotRequestType::Snapshot,
}, },
) { ) {
warn!( warn!(

View File

@ -34,7 +34,7 @@ pub mod commitment;
pub mod contains; pub mod contains;
pub mod cost_model; pub mod cost_model;
pub mod cost_tracker; pub mod cost_tracker;
mod epoch_accounts_hash; pub mod epoch_accounts_hash;
pub mod epoch_stakes; pub mod epoch_stakes;
pub mod execute_cost_table; pub mod execute_cost_table;
mod expected_rent_collection; mod expected_rent_collection;

View File

@ -32,6 +32,7 @@ pub type PendingSnapshotPackage = Arc<Mutex<Option<SnapshotPackage>>>;
#[derive(Debug)] #[derive(Debug)]
pub struct AccountsPackage { pub struct AccountsPackage {
pub package_type: AccountsPackageType,
pub slot: Slot, pub slot: Slot,
pub block_height: Slot, pub block_height: Slot,
pub slot_deltas: Vec<BankSlotDelta>, pub slot_deltas: Vec<BankSlotDelta>,
@ -44,7 +45,6 @@ pub struct AccountsPackage {
pub expected_capitalization: u64, pub expected_capitalization: u64,
pub accounts_hash_for_testing: Option<Hash>, pub accounts_hash_for_testing: Option<Hash>,
pub cluster_type: ClusterType, pub cluster_type: ClusterType,
pub snapshot_type: Option<SnapshotType>,
pub accounts: Arc<Accounts>, pub accounts: Arc<Accounts>,
pub epoch_schedule: EpochSchedule, pub epoch_schedule: EpochSchedule,
pub rent_collector: RentCollector, pub rent_collector: RentCollector,
@ -54,6 +54,7 @@ impl AccountsPackage {
/// Package up bank files, storages, and slot deltas for a snapshot /// Package up bank files, storages, and slot deltas for a snapshot
#[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_arguments)]
pub fn new( pub fn new(
package_type: AccountsPackageType,
bank: &Bank, bank: &Bank,
bank_snapshot_info: &BankSnapshotInfo, bank_snapshot_info: &BankSnapshotInfo,
bank_snapshots_dir: impl AsRef<Path>, bank_snapshots_dir: impl AsRef<Path>,
@ -64,22 +65,21 @@ impl AccountsPackage {
archive_format: ArchiveFormat, archive_format: ArchiveFormat,
snapshot_version: SnapshotVersion, snapshot_version: SnapshotVersion,
accounts_hash_for_testing: Option<Hash>, accounts_hash_for_testing: Option<Hash>,
snapshot_type: Option<SnapshotType>,
) -> Result<Self> { ) -> Result<Self> {
info!( if let AccountsPackageType::Snapshot(snapshot_type) = package_type {
"Package snapshot for bank {} has {} account storage entries (snapshot type: {:?})", info!(
bank.slot(), "Package snapshot for bank {} has {} account storage entries (snapshot type: {:?})",
snapshot_storages.len(), bank.slot(),
snapshot_type, snapshot_storages.len(),
); snapshot_type,
if let Some(SnapshotType::IncrementalSnapshot(incremental_snapshot_base_slot)) =
snapshot_type
{
assert!(
bank.slot() > incremental_snapshot_base_slot,
"Incremental snapshot base slot must be less than the bank being snapshotted!"
); );
if let SnapshotType::IncrementalSnapshot(incremental_snapshot_base_slot) = snapshot_type
{
assert!(
bank.slot() > incremental_snapshot_base_slot,
"Incremental snapshot base slot must be less than the bank being snapshotted!"
);
}
} }
// Hard link the snapshot into a tmpdir, to ensure its not removed prior to packaging. // Hard link the snapshot into a tmpdir, to ensure its not removed prior to packaging.
@ -100,6 +100,7 @@ impl AccountsPackage {
} }
Ok(Self { Ok(Self {
package_type,
slot: bank.slot(), slot: bank.slot(),
block_height: bank.block_height(), block_height: bank.block_height(),
slot_deltas, slot_deltas,
@ -114,7 +115,6 @@ impl AccountsPackage {
expected_capitalization: bank.capitalization(), expected_capitalization: bank.capitalization(),
accounts_hash_for_testing, accounts_hash_for_testing,
cluster_type: bank.cluster_type(), cluster_type: bank.cluster_type(),
snapshot_type,
accounts: bank.accounts(), accounts: bank.accounts(),
epoch_schedule: *bank.epoch_schedule(), epoch_schedule: *bank.epoch_schedule(),
rent_collector: bank.rent_collector().clone(), rent_collector: bank.rent_collector().clone(),
@ -122,6 +122,16 @@ impl AccountsPackage {
} }
} }
/// 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.
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub enum AccountsPackageType {
AccountsHashVerifier,
Snapshot(SnapshotType),
EpochAccountsHash,
}
pub struct SnapshotPackage { pub struct SnapshotPackage {
pub snapshot_archive_info: SnapshotArchiveInfo, pub snapshot_archive_info: SnapshotArchiveInfo,
pub block_height: Slot, pub block_height: Slot,
@ -134,40 +144,46 @@ pub struct SnapshotPackage {
impl SnapshotPackage { impl SnapshotPackage {
pub fn new(accounts_package: AccountsPackage, accounts_hash: Hash) -> Self { pub fn new(accounts_package: AccountsPackage, accounts_hash: Hash) -> Self {
assert!(
accounts_package.snapshot_type.is_some(),
"Cannot make a SnapshotPackage from an AccountsPackage when SnapshotType is None!"
);
let mut snapshot_storages = accounts_package.snapshot_storages; let mut snapshot_storages = accounts_package.snapshot_storages;
let snapshot_archive_path = match accounts_package.snapshot_type.unwrap() { let (snapshot_type, snapshot_archive_path) = match accounts_package.package_type {
SnapshotType::FullSnapshot => snapshot_utils::build_full_snapshot_archive_path( AccountsPackageType::Snapshot(snapshot_type) => match snapshot_type {
accounts_package.full_snapshot_archives_dir, SnapshotType::FullSnapshot => (
accounts_package.slot, snapshot_type,
&accounts_hash, snapshot_utils::build_full_snapshot_archive_path(
accounts_package.archive_format, accounts_package.full_snapshot_archives_dir,
), accounts_package.slot,
SnapshotType::IncrementalSnapshot(incremental_snapshot_base_slot) => { &accounts_hash,
snapshot_storages.retain(|storages| { accounts_package.archive_format,
storages ),
.first() // storages are grouped by slot in the outer Vec, so all storages will have the same slot as the first ),
.map(|storage| storage.slot() > incremental_snapshot_base_slot) SnapshotType::IncrementalSnapshot(incremental_snapshot_base_slot) => {
.unwrap_or_default() snapshot_storages.retain(|storages| {
}); storages
assert!( .first() // storages are grouped by slot in the outer Vec, so all storages will have the same slot as the first
snapshot_storages.iter().all(|storage| storage .map(|storage| storage.slot() > incremental_snapshot_base_slot)
.iter() .unwrap_or_default()
.all(|entry| entry.slot() > incremental_snapshot_base_slot)), });
"Incremental snapshot package must only contain storage entries where slot > incremental snapshot base slot (i.e. full snapshot slot)!" assert!(
snapshot_storages.iter().all(|storage| storage
.iter()
.all(|entry| entry.slot() > incremental_snapshot_base_slot)),
"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( (
accounts_package.incremental_snapshot_archives_dir, snapshot_type,
incremental_snapshot_base_slot, snapshot_utils::build_incremental_snapshot_archive_path(
accounts_package.slot, accounts_package.incremental_snapshot_archives_dir,
&accounts_hash, incremental_snapshot_base_slot,
accounts_package.archive_format, accounts_package.slot,
) &accounts_hash,
} accounts_package.archive_format,
),
)
}
},
_ => panic!(
"The AccountsPackage must be of type Snapshot in order to make a SnapshotPackage!"
),
}; };
Self { Self {
@ -182,7 +198,7 @@ impl SnapshotPackage {
snapshot_links: accounts_package.snapshot_links, snapshot_links: accounts_package.snapshot_links,
snapshot_storages, snapshot_storages,
snapshot_version: accounts_package.snapshot_version, snapshot_version: accounts_package.snapshot_version,
snapshot_type: accounts_package.snapshot_type.unwrap(), snapshot_type,
} }
} }
} }

View File

@ -17,7 +17,8 @@ use {
FullSnapshotArchiveInfo, IncrementalSnapshotArchiveInfo, SnapshotArchiveInfoGetter, FullSnapshotArchiveInfo, IncrementalSnapshotArchiveInfo, SnapshotArchiveInfoGetter,
}, },
snapshot_package::{ snapshot_package::{
AccountsPackage, PendingAccountsPackage, SnapshotPackage, SnapshotType, AccountsPackage, AccountsPackageType, PendingAccountsPackage, SnapshotPackage,
SnapshotType,
}, },
snapshot_utils::snapshot_storage_rebuilder::SnapshotStorageRebuilder, snapshot_utils::snapshot_storage_rebuilder::SnapshotStorageRebuilder,
status_cache, status_cache,
@ -2099,7 +2100,7 @@ pub fn snapshot_bank(
snapshot_version: SnapshotVersion, snapshot_version: SnapshotVersion,
archive_format: ArchiveFormat, archive_format: ArchiveFormat,
hash_for_testing: Option<Hash>, hash_for_testing: Option<Hash>,
snapshot_type: Option<SnapshotType>, accounts_package_type: AccountsPackageType,
) -> Result<()> { ) -> Result<()> {
let snapshot_storages = get_snapshot_storages(root_bank); let snapshot_storages = get_snapshot_storages(root_bank);
@ -2114,6 +2115,7 @@ pub fn snapshot_bank(
inc_new_counter_info!("add-snapshot-ms", add_snapshot_time.as_ms() as usize); inc_new_counter_info!("add-snapshot-ms", add_snapshot_time.as_ms() as usize);
let accounts_package = AccountsPackage::new( let accounts_package = AccountsPackage::new(
accounts_package_type,
root_bank, root_bank,
&bank_snapshot_info, &bank_snapshot_info,
bank_snapshots_dir, bank_snapshots_dir,
@ -2124,7 +2126,6 @@ pub fn snapshot_bank(
archive_format, archive_format,
snapshot_version, snapshot_version,
hash_for_testing, hash_for_testing,
snapshot_type,
) )
.expect("failed to hard link bank snapshot into a tmpdir"); .expect("failed to hard link bank snapshot into a tmpdir");
@ -2133,17 +2134,18 @@ pub fn snapshot_bank(
{ {
let mut pending_accounts_package = pending_accounts_package.lock().unwrap(); let mut pending_accounts_package = pending_accounts_package.lock().unwrap();
if can_submit_accounts_package(&accounts_package, pending_accounts_package.as_ref()) { if can_submit_accounts_package(&accounts_package, pending_accounts_package.as_ref()) {
let package_type = accounts_package.package_type;
let old_accounts_package = pending_accounts_package.replace(accounts_package); let old_accounts_package = pending_accounts_package.replace(accounts_package);
drop(pending_accounts_package); drop(pending_accounts_package);
if let Some(old_accounts_package) = old_accounts_package { if let Some(old_accounts_package) = old_accounts_package {
debug!( info!(
"The pending AccountsPackage has been overwritten: \ "The pending AccountsPackage has been overwritten: \
\nNew AccountsPackage slot: {}, snapshot type: {:?} \ \nNew AccountsPackage slot: {}, package type: {:?} \
\nOld AccountsPackage slot: {}, snapshot type: {:?}", \nOld AccountsPackage slot: {}, package type: {:?}",
root_bank.slot(), root_bank.slot(),
snapshot_type, package_type,
old_accounts_package.slot, old_accounts_package.slot,
old_accounts_package.snapshot_type, old_accounts_package.package_type,
); );
} }
} }
@ -2279,6 +2281,7 @@ pub fn package_and_archive_full_snapshot(
) -> Result<FullSnapshotArchiveInfo> { ) -> Result<FullSnapshotArchiveInfo> {
let slot_deltas = bank.status_cache.read().unwrap().root_slot_deltas(); let slot_deltas = bank.status_cache.read().unwrap().root_slot_deltas();
let accounts_package = AccountsPackage::new( let accounts_package = AccountsPackage::new(
AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
bank, bank,
bank_snapshot_info, bank_snapshot_info,
bank_snapshots_dir, bank_snapshots_dir,
@ -2289,7 +2292,6 @@ pub fn package_and_archive_full_snapshot(
archive_format, archive_format,
snapshot_version, snapshot_version,
None, None,
Some(SnapshotType::FullSnapshot),
)?; )?;
crate::serde_snapshot::reserialize_bank_with_new_accounts_hash( crate::serde_snapshot::reserialize_bank_with_new_accounts_hash(
@ -2331,6 +2333,9 @@ pub fn package_and_archive_incremental_snapshot(
) -> Result<IncrementalSnapshotArchiveInfo> { ) -> Result<IncrementalSnapshotArchiveInfo> {
let slot_deltas = bank.status_cache.read().unwrap().root_slot_deltas(); let slot_deltas = bank.status_cache.read().unwrap().root_slot_deltas();
let accounts_package = AccountsPackage::new( let accounts_package = AccountsPackage::new(
AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(
incremental_snapshot_base_slot,
)),
bank, bank,
bank_snapshot_info, bank_snapshot_info,
bank_snapshots_dir, bank_snapshots_dir,
@ -2341,9 +2346,6 @@ pub fn package_and_archive_incremental_snapshot(
archive_format, archive_format,
snapshot_version, snapshot_version,
None, None,
Some(SnapshotType::IncrementalSnapshot(
incremental_snapshot_base_slot,
)),
)?; )?;
crate::serde_snapshot::reserialize_bank_with_new_accounts_hash( crate::serde_snapshot::reserialize_bank_with_new_accounts_hash(
@ -2389,6 +2391,7 @@ pub fn should_take_incremental_snapshot(
/// ///
/// If there's no pending accounts package, then submit /// If there's no pending accounts package, then submit
/// Otherwise, check if the pending accounts package can be overwritten /// Otherwise, check if the pending accounts package can be overwritten
#[must_use]
fn can_submit_accounts_package( fn can_submit_accounts_package(
accounts_package: &AccountsPackage, accounts_package: &AccountsPackage,
pending_accounts_package: Option<&AccountsPackage>, pending_accounts_package: Option<&AccountsPackage>,
@ -2402,23 +2405,39 @@ fn can_submit_accounts_package(
/// Decide when it is appropriate to overwrite a pending accounts package /// Decide when it is appropriate to overwrite a pending accounts package
/// ///
/// This is based on the values for `snapshot_type` in both the `accounts_package` and the /// The priority of the package types is (from highest to lowest):
/// `pending_accounts_package`: /// 1. Epoch Account Hash
/// - if the new AccountsPackage is for a full snapshot, always overwrite /// 2. Full Snapshot
/// - if the new AccountsPackage is for an incremental snapshot, overwrite as long as there isn't a /// 3. Incremental Snapshot
/// pending full snapshot /// 4. Accounts Hash Verifier
/// - otherwise, only overwrite if the pending package's snapshot type is None ///
/// New packages of higher priority types can overwrite pending packages of *equivalent or lower*
/// priority types.
#[must_use]
fn can_overwrite_pending_accounts_package( fn can_overwrite_pending_accounts_package(
accounts_package: &AccountsPackage, accounts_package: &AccountsPackage,
pending_accounts_package: &AccountsPackage, pending_accounts_package: &AccountsPackage,
) -> bool { ) -> bool {
match accounts_package.snapshot_type { match (
Some(SnapshotType::FullSnapshot) => true, &pending_accounts_package.package_type,
Some(SnapshotType::IncrementalSnapshot(_)) => pending_accounts_package &accounts_package.package_type,
.snapshot_type ) {
.map(|snapshot_type| !snapshot_type.is_full_snapshot()) (AccountsPackageType::EpochAccountsHash, AccountsPackageType::EpochAccountsHash) => {
.unwrap_or(true), panic!(
None => pending_accounts_package.snapshot_type.is_none(), "Both pending and new accounts packages are of type EpochAccountsHash! \
EAH calculations must complete before new ones are enqueued. \
\npending accounts package slot: {} \
\n new accounts package slot: {}",
pending_accounts_package.slot, accounts_package.slot,
);
}
(_, AccountsPackageType::EpochAccountsHash) => true,
(AccountsPackageType::EpochAccountsHash, _) => false,
(_, AccountsPackageType::Snapshot(SnapshotType::FullSnapshot)) => true,
(AccountsPackageType::Snapshot(SnapshotType::FullSnapshot), _) => false,
(_, AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(_))) => true,
(AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(_)), _) => false,
_ => true,
} }
} }
@ -2426,7 +2445,10 @@ fn can_overwrite_pending_accounts_package(
mod tests { mod tests {
use { use {
super::*, super::*,
crate::{accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING, status_cache::Status}, crate::{
accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING, snapshot_package::AccountsPackageType,
status_cache::Status,
},
assert_matches::assert_matches, assert_matches::assert_matches,
bincode::{deserialize_from, serialize_into}, bincode::{deserialize_from, serialize_into},
solana_sdk::{ solana_sdk::{
@ -4012,91 +4034,162 @@ mod tests {
assert_eq!(bank_fields.parent_slot, bank2.parent_slot()); assert_eq!(bank_fields.parent_slot, bank2.parent_slot());
} }
/// All the permutations of `snapshot_type` for the new-and-old accounts packages: /// Test `can_submit_accounts_packages()` with all permutations of `package_type`
/// for both the pending accounts package and the new accounts package.
/// ///
/// new | old | /// pending | new |
/// snapshot | snapshot | /// package | package | result
/// type | type | result /// ---------+---------+--------
/// ----------+----------+-------- /// 1. None | X | true
/// FSS | FSS | true /// 2. AHV | AHV | true
/// FSS | ISS | true /// 3. AHV | ISS | true
/// FSS | None | true /// 4. AHV | FSS | true
/// ISS | FSS | false /// 5. AHV | EAH | true
/// ISS | ISS | true /// 6. ISS | AHV | false
/// ISS | None | true /// 7. ISS | ISS | true
/// None | FSS | false /// 8. ISS | FSS | true
/// None | ISS | false /// 9. ISS | EAH | true
/// None | None | true /// 10. FSS | AHV | false
/// 11. FSS | ISS | false
/// 12. FSS | FSS | true
/// 13. FSS | EAH | true
/// 14. EAH | AHV | false
/// 15. EAH | ISS | false
/// 16. EAH | FSS | false
/// 17. EAH | EAH | assert unreachable, so test separately
#[test] #[test]
fn test_can_submit_accounts_package() { fn test_can_submit_accounts_package() {
/// helper function to create an AccountsPackage that's good enough for this test // Test 1
fn new_accounts_package_with(snapshot_type: Option<SnapshotType>) -> AccountsPackage {
AccountsPackage {
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: None,
cluster_type: solana_sdk::genesis_config::ClusterType::Development,
snapshot_type,
accounts: Arc::new(crate::accounts::Accounts::default_for_tests()),
epoch_schedule: solana_sdk::epoch_schedule::EpochSchedule::default(),
rent_collector: crate::rent_collector::RentCollector::default(),
}
}
for (new_snapshot_type, old_snapshot_type, expected_result) in [
(
Some(SnapshotType::FullSnapshot),
Some(SnapshotType::FullSnapshot),
true,
),
(
Some(SnapshotType::FullSnapshot),
Some(SnapshotType::IncrementalSnapshot(0)),
true,
),
(Some(SnapshotType::FullSnapshot), None, true),
(
Some(SnapshotType::IncrementalSnapshot(0)),
Some(SnapshotType::FullSnapshot),
false,
),
(
Some(SnapshotType::IncrementalSnapshot(0)),
Some(SnapshotType::IncrementalSnapshot(0)),
true,
),
(Some(SnapshotType::IncrementalSnapshot(0)), None, true),
(None, Some(SnapshotType::FullSnapshot), false),
(None, Some(SnapshotType::IncrementalSnapshot(0)), false),
(None, None, true),
] {
let new_accounts_package = new_accounts_package_with(new_snapshot_type);
let old_accounts_package = new_accounts_package_with(old_snapshot_type);
let actual_result = can_overwrite_pending_accounts_package(
&new_accounts_package,
&old_accounts_package,
);
assert_eq!(expected_result, actual_result);
}
// Also test when the pending package is None
{ {
let accounts_package = new_accounts_package_with(None);
let pending_accounts_package = None; let pending_accounts_package = None;
let accounts_package = new_accounts_package(AccountsPackageType::AccountsHashVerifier);
assert!(can_submit_accounts_package( assert!(can_submit_accounts_package(
&accounts_package, &accounts_package,
pending_accounts_package pending_accounts_package
)); ));
} }
for (pending_package_type, new_package_type, expected_result) in [
// Tests 2-5, pending package is AHV
(
AccountsPackageType::AccountsHashVerifier,
AccountsPackageType::AccountsHashVerifier,
true,
),
(
AccountsPackageType::AccountsHashVerifier,
AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(0)),
true,
),
(
AccountsPackageType::AccountsHashVerifier,
AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
true,
),
(
AccountsPackageType::AccountsHashVerifier,
AccountsPackageType::EpochAccountsHash,
true,
),
// Tests 6-9, pending package is ISS
(
AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(0)),
AccountsPackageType::AccountsHashVerifier,
false,
),
(
AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(0)),
AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(0)),
true,
),
(
AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(0)),
AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
true,
),
(
AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(0)),
AccountsPackageType::EpochAccountsHash,
true,
),
// Tests 10-13, pending package is FSS
(
AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
AccountsPackageType::AccountsHashVerifier,
false,
),
(
AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(0)),
false,
),
(
AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
true,
),
(
AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
AccountsPackageType::EpochAccountsHash,
true,
),
// Tests 14-16, pending package is EAH
(
AccountsPackageType::EpochAccountsHash,
AccountsPackageType::AccountsHashVerifier,
false,
),
(
AccountsPackageType::EpochAccountsHash,
AccountsPackageType::Snapshot(SnapshotType::IncrementalSnapshot(0)),
false,
),
(
AccountsPackageType::EpochAccountsHash,
AccountsPackageType::Snapshot(SnapshotType::FullSnapshot),
false,
),
// tested separately: (AccountsPackageType::EpochAccountsHash, None, AccountsPackageType::EpochAccountsHash, None, X),
] {
let pending_accounts_package = new_accounts_package(pending_package_type);
let accounts_package = new_accounts_package(new_package_type);
let actual_result =
can_submit_accounts_package(&accounts_package, Some(&pending_accounts_package));
assert_eq!(expected_result, actual_result);
}
}
/// It should not be allowed to have a new accounts package intended for EAH when there is
/// already a pending EAH accounts package.
#[test]
#[should_panic]
fn test_can_submit_accounts_package_both_are_eah() {
let pending_accounts_package = new_accounts_package(AccountsPackageType::EpochAccountsHash);
let accounts_package = new_accounts_package(AccountsPackageType::EpochAccountsHash);
_ = can_submit_accounts_package(&accounts_package, Some(&pending_accounts_package));
}
/// helper function to create an AccountsPackage that's good enough for tests
fn new_accounts_package(package_type: AccountsPackageType) -> AccountsPackage {
AccountsPackage {
package_type,
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: None,
cluster_type: solana_sdk::genesis_config::ClusterType::Development,
accounts: Arc::new(crate::accounts::Accounts::default_for_tests()),
epoch_schedule: solana_sdk::epoch_schedule::EpochSchedule::default(),
rent_collector: crate::rent_collector::RentCollector::default(),
}
} }
#[test] #[test]