move hash calculation out of acct bg svc (#23689)
* move hash calculation out of acct bg svc * pr feedback
This commit is contained in:
committed by
GitHub
parent
cb1507126f
commit
210f6a6fab
@ -96,7 +96,7 @@ impl AccountsHashVerifier {
|
||||
fault_injection_rate_slots: u64,
|
||||
snapshot_config: Option<&SnapshotConfig>,
|
||||
) {
|
||||
Self::verify_accounts_package_hash(&accounts_package);
|
||||
let accounts_hash = Self::calculate_and_verify_accounts_hash(&accounts_package);
|
||||
|
||||
Self::push_accounts_hashes_to_cluster(
|
||||
&accounts_package,
|
||||
@ -106,55 +106,63 @@ impl AccountsHashVerifier {
|
||||
hashes,
|
||||
exit,
|
||||
fault_injection_rate_slots,
|
||||
accounts_hash,
|
||||
);
|
||||
|
||||
Self::submit_for_packaging(accounts_package, pending_snapshot_package, snapshot_config);
|
||||
Self::submit_for_packaging(
|
||||
accounts_package,
|
||||
pending_snapshot_package,
|
||||
snapshot_config,
|
||||
accounts_hash,
|
||||
);
|
||||
}
|
||||
|
||||
fn verify_accounts_package_hash(accounts_package: &AccountsPackage) {
|
||||
/// returns calculated accounts hash
|
||||
fn calculate_and_verify_accounts_hash(accounts_package: &AccountsPackage) -> Hash {
|
||||
let mut measure_hash = Measure::start("hash");
|
||||
let mut sort_time = Measure::start("sort_storages");
|
||||
let sorted_storages = SortedStorages::new(&accounts_package.snapshot_storages);
|
||||
sort_time.stop();
|
||||
|
||||
let mut timings = HashStats {
|
||||
storage_sort_us: sort_time.as_us(),
|
||||
..HashStats::default()
|
||||
};
|
||||
timings.calc_storage_size_quartiles(&accounts_package.snapshot_storages);
|
||||
|
||||
let (accounts_hash, lamports) = accounts_package
|
||||
.accounts
|
||||
.accounts_db
|
||||
.calculate_accounts_hash_without_index(
|
||||
&CalcAccountsHashConfig {
|
||||
use_bg_thread_pool: true,
|
||||
check_hash: false,
|
||||
ancestors: None,
|
||||
use_write_cache: false,
|
||||
epoch_schedule: &accounts_package.epoch_schedule,
|
||||
rent_collector: &accounts_package.rent_collector,
|
||||
},
|
||||
&sorted_storages,
|
||||
timings,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(accounts_package.expected_capitalization, lamports);
|
||||
if let Some(expected_hash) = accounts_package.accounts_hash_for_testing {
|
||||
let mut sort_time = Measure::start("sort_storages");
|
||||
let sorted_storages = SortedStorages::new(&accounts_package.snapshot_storages);
|
||||
sort_time.stop();
|
||||
|
||||
let mut timings = HashStats {
|
||||
storage_sort_us: sort_time.as_us(),
|
||||
..HashStats::default()
|
||||
};
|
||||
timings.calc_storage_size_quartiles(&accounts_package.snapshot_storages);
|
||||
|
||||
let (hash, lamports) = accounts_package
|
||||
.accounts
|
||||
.accounts_db
|
||||
.calculate_accounts_hash_without_index(
|
||||
&CalcAccountsHashConfig {
|
||||
use_bg_thread_pool: true,
|
||||
check_hash: false,
|
||||
ancestors: None,
|
||||
use_write_cache: false,
|
||||
epoch_schedule: &accounts_package.epoch_schedule,
|
||||
rent_collector: &accounts_package.rent_collector,
|
||||
},
|
||||
&sorted_storages,
|
||||
timings,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(accounts_package.expected_capitalization, lamports);
|
||||
assert_eq!(expected_hash, hash);
|
||||
assert_eq!(expected_hash, accounts_hash);
|
||||
};
|
||||
|
||||
measure_hash.stop();
|
||||
solana_runtime::serde_snapshot::reserialize_bank_with_new_accounts_hash(
|
||||
accounts_package.snapshot_links.path(),
|
||||
accounts_package.slot,
|
||||
&accounts_package.accounts_hash,
|
||||
&accounts_hash,
|
||||
);
|
||||
datapoint_info!(
|
||||
"accounts_hash_verifier",
|
||||
("calculate_hash", measure_hash.as_us(), i64),
|
||||
);
|
||||
accounts_hash
|
||||
}
|
||||
|
||||
fn push_accounts_hashes_to_cluster(
|
||||
@ -165,8 +173,8 @@ impl AccountsHashVerifier {
|
||||
hashes: &mut Vec<(Slot, Hash)>,
|
||||
exit: &Arc<AtomicBool>,
|
||||
fault_injection_rate_slots: u64,
|
||||
accounts_hash: Hash,
|
||||
) {
|
||||
let hash = accounts_package.accounts_hash;
|
||||
if fault_injection_rate_slots != 0
|
||||
&& accounts_package.slot % fault_injection_rate_slots == 0
|
||||
{
|
||||
@ -177,10 +185,10 @@ impl AccountsHashVerifier {
|
||||
};
|
||||
warn!("inserting fault at slot: {}", accounts_package.slot);
|
||||
let rand = thread_rng().gen_range(0, 10);
|
||||
let hash = extend_and_hash(&hash, &[rand]);
|
||||
let hash = extend_and_hash(&accounts_hash, &[rand]);
|
||||
hashes.push((accounts_package.slot, hash));
|
||||
} else {
|
||||
hashes.push((accounts_package.slot, hash));
|
||||
hashes.push((accounts_package.slot, accounts_hash));
|
||||
}
|
||||
|
||||
while hashes.len() > MAX_SNAPSHOT_HASHES {
|
||||
@ -204,6 +212,7 @@ impl AccountsHashVerifier {
|
||||
accounts_package: AccountsPackage,
|
||||
pending_snapshot_package: Option<&PendingSnapshotPackage>,
|
||||
snapshot_config: Option<&SnapshotConfig>,
|
||||
accounts_hash: Hash,
|
||||
) {
|
||||
if accounts_package.snapshot_type.is_none()
|
||||
|| pending_snapshot_package.is_none()
|
||||
@ -212,7 +221,7 @@ impl AccountsHashVerifier {
|
||||
return;
|
||||
};
|
||||
|
||||
let snapshot_package = SnapshotPackage::from(accounts_package);
|
||||
let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash);
|
||||
let pending_snapshot_package = pending_snapshot_package.unwrap();
|
||||
let _snapshot_config = snapshot_config.unwrap();
|
||||
|
||||
@ -301,6 +310,7 @@ mod tests {
|
||||
sysvar::epoch_schedule::EpochSchedule,
|
||||
},
|
||||
solana_streamer::socket::SocketAddrSpace,
|
||||
std::str::FromStr,
|
||||
};
|
||||
|
||||
fn new_test_cluster_info(contact_info: ContactInfo) -> ClusterInfo {
|
||||
@ -364,6 +374,7 @@ mod tests {
|
||||
..SnapshotConfig::default()
|
||||
};
|
||||
let accounts = Arc::new(solana_runtime::accounts::Accounts::default_for_tests());
|
||||
let expected_hash = Hash::from_str("GKot5hBsd81kMupNCXHaqbhv3huEbxAFMLnpcX2hniwn").unwrap();
|
||||
for i in 0..MAX_SNAPSHOT_HASHES + 1 {
|
||||
let accounts_package = AccountsPackage {
|
||||
slot: full_snapshot_archive_interval_slots + i as u64,
|
||||
@ -371,7 +382,6 @@ mod tests {
|
||||
slot_deltas: vec![],
|
||||
snapshot_links: TempDir::new().unwrap(),
|
||||
snapshot_storages: vec![],
|
||||
accounts_hash: hash(&[i as u8]),
|
||||
archive_format: ArchiveFormat::TarBzip2,
|
||||
snapshot_version: SnapshotVersion::default(),
|
||||
snapshot_archives_dir: PathBuf::default(),
|
||||
@ -409,13 +419,13 @@ mod tests {
|
||||
assert_eq!(cluster_hashes.len(), MAX_SNAPSHOT_HASHES);
|
||||
assert_eq!(
|
||||
cluster_hashes[0],
|
||||
(full_snapshot_archive_interval_slots + 1, hash(&[1]))
|
||||
(full_snapshot_archive_interval_slots + 1, expected_hash)
|
||||
);
|
||||
assert_eq!(
|
||||
cluster_hashes[MAX_SNAPSHOT_HASHES - 1],
|
||||
(
|
||||
full_snapshot_archive_interval_slots + MAX_SNAPSHOT_HASHES as u64,
|
||||
hash(&[MAX_SNAPSHOT_HASHES as u8])
|
||||
expected_hash
|
||||
)
|
||||
);
|
||||
}
|
||||
|
@ -288,7 +288,8 @@ mod tests {
|
||||
accounts_package.slot,
|
||||
&last_bank.get_accounts_hash(),
|
||||
);
|
||||
let snapshot_package = SnapshotPackage::from(accounts_package);
|
||||
let snapshot_package =
|
||||
SnapshotPackage::new(accounts_package, last_bank.get_accounts_hash());
|
||||
snapshot_utils::archive_snapshot_package(
|
||||
&snapshot_package,
|
||||
snapshot_config.maximum_full_snapshot_archives_to_retain,
|
||||
@ -391,7 +392,6 @@ mod tests {
|
||||
let tx = system_transaction::transfer(mint_keypair, &key1, 1, genesis_config.hash());
|
||||
assert_eq!(bank.process_transaction(&tx), Ok(()));
|
||||
bank.squash();
|
||||
let accounts_hash = bank.update_accounts_hash();
|
||||
|
||||
let pending_accounts_package = {
|
||||
if slot == saved_slot as u64 {
|
||||
@ -461,7 +461,9 @@ mod tests {
|
||||
saved_archive_path = Some(snapshot_utils::build_full_snapshot_archive_path(
|
||||
snapshot_archives_dir,
|
||||
slot,
|
||||
&accounts_hash,
|
||||
// this needs to match the hash value that we reserialize with later. It is complicated, so just use default.
|
||||
// This hash value is just used to build the file name. Since this is mocked up test code, it is sufficient to pass default here.
|
||||
&Hash::default(),
|
||||
ArchiveFormat::TarBzip2,
|
||||
));
|
||||
}
|
||||
@ -515,7 +517,7 @@ mod tests {
|
||||
accounts_package.slot,
|
||||
&Hash::default(),
|
||||
);
|
||||
let snapshot_package = SnapshotPackage::from(accounts_package);
|
||||
let snapshot_package = SnapshotPackage::new(accounts_package, Hash::default());
|
||||
pending_snapshot_package
|
||||
.lock()
|
||||
.unwrap()
|
||||
|
@ -4,6 +4,7 @@
|
||||
|
||||
use {
|
||||
crate::{
|
||||
accounts_hash::CalcAccountsHashConfig,
|
||||
bank::{Bank, BankSlotDelta, DropCallback},
|
||||
bank_forks::BankForks,
|
||||
snapshot_config::SnapshotConfig,
|
||||
@ -109,7 +110,7 @@ impl SnapshotRequestHandler {
|
||||
|
||||
let previous_hash = if test_hash_calculation {
|
||||
// We have to use the index version here.
|
||||
// We cannot calculate the non-index way because cache has not been flushed and stores don't match reality.
|
||||
// We cannot calculate the non-index way because cache has not been flushed and stores don't match reality. This comment is out of date and can be re-evaluated.
|
||||
snapshot_root_bank.update_accounts_hash_with_index_option(true, false, false)
|
||||
} else {
|
||||
Hash::default()
|
||||
@ -144,20 +145,27 @@ impl SnapshotRequestHandler {
|
||||
}
|
||||
flush_accounts_cache_time.stop();
|
||||
|
||||
let use_index_hash_calculation = false;
|
||||
let mut hash_time = Measure::start("hash_time");
|
||||
let this_hash = snapshot_root_bank.update_accounts_hash_with_index_option(
|
||||
use_index_hash_calculation,
|
||||
test_hash_calculation,
|
||||
false,
|
||||
);
|
||||
let hash_for_testing = if test_hash_calculation {
|
||||
let use_index_hash_calculation = false;
|
||||
let check_hash = false;
|
||||
|
||||
let (this_hash, _cap) = snapshot_root_bank.accounts().accounts_db.calculate_accounts_hash_helper(
|
||||
use_index_hash_calculation,
|
||||
snapshot_root_bank.slot(),
|
||||
&CalcAccountsHashConfig {
|
||||
use_bg_thread_pool: true,
|
||||
check_hash,
|
||||
ancestors: None,
|
||||
use_write_cache: false,
|
||||
epoch_schedule: snapshot_root_bank.epoch_schedule(),
|
||||
rent_collector: &snapshot_root_bank.rent_collector(),
|
||||
},
|
||||
).unwrap();
|
||||
assert_eq!(previous_hash, this_hash);
|
||||
Some(snapshot_root_bank.get_accounts_hash())
|
||||
Some(this_hash)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
hash_time.stop();
|
||||
|
||||
let mut clean_time = Measure::start("clean_time");
|
||||
// Don't clean the slot we're snapshotting because it may have zero-lamport
|
||||
@ -234,7 +242,6 @@ impl SnapshotRequestHandler {
|
||||
|
||||
datapoint_info!(
|
||||
"handle_snapshot_requests-timing",
|
||||
("hash_time", hash_time.as_us(), i64),
|
||||
(
|
||||
"flush_accounts_cache_time",
|
||||
flush_accounts_cache_time.as_us(),
|
||||
|
@ -5505,7 +5505,7 @@ impl AccountsDb {
|
||||
}
|
||||
}
|
||||
|
||||
fn calculate_accounts_hash_helper(
|
||||
pub(crate) fn calculate_accounts_hash_helper(
|
||||
&self,
|
||||
use_index: bool,
|
||||
slot: Slot,
|
||||
|
@ -37,7 +37,6 @@ pub struct AccountsPackage {
|
||||
pub slot_deltas: Vec<BankSlotDelta>,
|
||||
pub snapshot_links: TempDir,
|
||||
pub snapshot_storages: SnapshotStorages,
|
||||
pub accounts_hash: Hash, // temporarily here while we still have to calculate hash before serializing bank
|
||||
pub archive_format: ArchiveFormat,
|
||||
pub snapshot_version: SnapshotVersion,
|
||||
pub snapshot_archives_dir: PathBuf,
|
||||
@ -104,7 +103,6 @@ impl AccountsPackage {
|
||||
slot_deltas,
|
||||
snapshot_links,
|
||||
snapshot_storages,
|
||||
accounts_hash: bank.get_accounts_hash(),
|
||||
archive_format,
|
||||
snapshot_version,
|
||||
snapshot_archives_dir: snapshot_archives_dir.as_ref().to_path_buf(),
|
||||
@ -129,8 +127,8 @@ pub struct SnapshotPackage {
|
||||
pub snapshot_type: SnapshotType,
|
||||
}
|
||||
|
||||
impl From<AccountsPackage> for SnapshotPackage {
|
||||
fn from(accounts_package: AccountsPackage) -> Self {
|
||||
impl SnapshotPackage {
|
||||
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!"
|
||||
@ -141,7 +139,7 @@ impl From<AccountsPackage> for SnapshotPackage {
|
||||
SnapshotType::FullSnapshot => snapshot_utils::build_full_snapshot_archive_path(
|
||||
accounts_package.snapshot_archives_dir,
|
||||
accounts_package.slot,
|
||||
&accounts_package.accounts_hash,
|
||||
&accounts_hash,
|
||||
accounts_package.archive_format,
|
||||
),
|
||||
SnapshotType::IncrementalSnapshot(incremental_snapshot_base_slot) => {
|
||||
@ -161,7 +159,7 @@ impl From<AccountsPackage> for SnapshotPackage {
|
||||
accounts_package.snapshot_archives_dir,
|
||||
incremental_snapshot_base_slot,
|
||||
accounts_package.slot,
|
||||
&accounts_package.accounts_hash,
|
||||
&accounts_hash,
|
||||
accounts_package.archive_format,
|
||||
)
|
||||
}
|
||||
@ -171,7 +169,7 @@ impl From<AccountsPackage> for SnapshotPackage {
|
||||
snapshot_archive_info: SnapshotArchiveInfo {
|
||||
path: snapshot_archive_path,
|
||||
slot: accounts_package.slot,
|
||||
hash: accounts_package.accounts_hash,
|
||||
hash: accounts_hash,
|
||||
archive_format: accounts_package.archive_format,
|
||||
},
|
||||
block_height: accounts_package.block_height,
|
||||
|
@ -1924,7 +1924,7 @@ pub fn package_and_archive_full_snapshot(
|
||||
&bank.get_accounts_hash(),
|
||||
);
|
||||
|
||||
let snapshot_package = SnapshotPackage::from(accounts_package);
|
||||
let snapshot_package = SnapshotPackage::new(accounts_package, bank.get_accounts_hash());
|
||||
archive_snapshot_package(
|
||||
&snapshot_package,
|
||||
maximum_full_snapshot_archives_to_retain,
|
||||
@ -1971,7 +1971,7 @@ pub fn package_and_archive_incremental_snapshot(
|
||||
&bank.get_accounts_hash(),
|
||||
);
|
||||
|
||||
let snapshot_package = SnapshotPackage::from(accounts_package);
|
||||
let snapshot_package = SnapshotPackage::new(accounts_package, bank.get_accounts_hash());
|
||||
archive_snapshot_package(
|
||||
&snapshot_package,
|
||||
maximum_full_snapshot_archives_to_retain,
|
||||
@ -3443,7 +3443,6 @@ mod tests {
|
||||
slot_deltas: Vec::default(),
|
||||
snapshot_links: TempDir::new().unwrap(),
|
||||
snapshot_storages: SnapshotStorages::default(),
|
||||
accounts_hash: Hash::default(),
|
||||
archive_format: ArchiveFormat::Tar,
|
||||
snapshot_version: SnapshotVersion::default(),
|
||||
snapshot_archives_dir: PathBuf::default(),
|
||||
|
Reference in New Issue
Block a user