Skip to content

Commit

Permalink
acct idx can no longer use write cache (solana-labs#28150)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeffwashington
jeffwashington authored Sep 30, 2022
1 parent 8255822 commit cfc124c
Show file tree
Hide file tree
Showing 9 changed files with 7 additions and 80 deletions.
1 change: 0 additions & 1 deletion accounts-bench/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ fn main() {
solana_sdk::clock::Slot::default(),
&ancestors,
None,
false,
&EpochSchedule::default(),
&RentCollector::default(),
false,
Expand Down
3 changes: 0 additions & 3 deletions core/src/accounts_hash_verifier.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ impl AccountsHashVerifier {
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,
store_detailed_debug_info_on_failure: false,
Expand All @@ -171,7 +170,6 @@ impl AccountsHashVerifier {
use_bg_thread_pool: false,
check_hash: false,
ancestors: None,
use_write_cache: false,
epoch_schedule: &accounts_package.epoch_schedule,
rent_collector: &accounts_package.rent_collector,
store_detailed_debug_info_on_failure: false,
Expand All @@ -191,7 +189,6 @@ impl AccountsHashVerifier {
use_bg_thread_pool: false,
check_hash: false,
ancestors: None,
use_write_cache: false,
epoch_schedule: &accounts_package.epoch_schedule,
rent_collector: &accounts_package.rent_collector,
// now that we've failed, store off the failing contents that produced a bad capitalization
Expand Down
2 changes: 0 additions & 2 deletions ledger/src/blockstore_processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1545,11 +1545,9 @@ fn load_frozen_forks(

fn run_final_hash_calc(bank: &Bank, on_halt_store_hash_raw_data_for_debug: bool) {
bank.force_flush_accounts_cache();
let can_cached_slot_be_unflushed = false;
// note that this slot may not be a root
let _ = bank.verify_bank_hash(VerifyBankHash {
test_hash_calculation: false,
can_cached_slot_be_unflushed,
ignore_mismatch: true,
require_rooted_bank: false,
run_in_background: false,
Expand Down
1 change: 0 additions & 1 deletion runtime/benches/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ fn test_accounts_hash_bank_hash(bencher: &mut Bencher) {
&RentCollector::default(),
false,
false,
false,
true,
))
});
Expand Down
4 changes: 0 additions & 4 deletions runtime/src/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,6 @@ impl Accounts {
&self,
ancestors: &Ancestors,
slot: Slot,
can_cached_slot_be_unflushed: bool,
debug_verify: bool,
epoch_schedule: &EpochSchedule,
rent_collector: &RentCollector,
Expand All @@ -832,7 +831,6 @@ impl Accounts {
slot,
ancestors,
None,
can_cached_slot_be_unflushed,
epoch_schedule,
rent_collector,
is_startup,
Expand All @@ -852,7 +850,6 @@ impl Accounts {
test_hash_calculation: bool,
epoch_schedule: &EpochSchedule,
rent_collector: &RentCollector,
can_cached_slot_be_unflushed: bool,
ignore_mismatch: bool,
store_detailed_debug_info: bool,
enable_rehashing: bool,
Expand All @@ -864,7 +861,6 @@ impl Accounts {
test_hash_calculation,
epoch_schedule,
rent_collector,
can_cached_slot_be_unflushed,
ignore_mismatch,
store_detailed_debug_info,
enable_rehashing,
Expand Down
1 change: 0 additions & 1 deletion runtime/src/accounts_background_service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ impl SnapshotRequestHandler {
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(),
store_detailed_debug_info_on_failure: false,
Expand Down
58 changes: 4 additions & 54 deletions runtime/src/accounts_db.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6715,7 +6715,6 @@ impl AccountsDb {
slot,
ancestors,
None,
false,
epoch_schedule,
rent_collector,
false,
Expand All @@ -6731,7 +6730,6 @@ impl AccountsDb {
slot,
ancestors,
None,
false,
&EpochSchedule::default(),
&RentCollector::default(),
false,
Expand Down Expand Up @@ -6935,9 +6933,8 @@ impl AccountsDb {
// if we're using the write cache, then we can't rely on cached append vecs since the append vecs may not include every account
// Single cached slots get cached and full chunks get cached.
// chunks that don't divide evenly would include some cached append vecs that are no longer part of this range and some that are, so we have to ignore caching on non-evenly dividing chunks.
let eligible_for_caching = !config.use_write_cache
&& (single_cached_slot
|| end_exclusive.saturating_sub(start) == MAX_ITEMS_PER_CHUNK);
let eligible_for_caching = single_cached_slot
|| end_exclusive.saturating_sub(start) == MAX_ITEMS_PER_CHUNK;

if eligible_for_caching || config.store_detailed_debug_info_on_failure {
let range = bin_range.end - bin_range.start;
Expand All @@ -6954,7 +6951,6 @@ impl AccountsDb {
.saturating_sub(slots_per_epoch);

let mut file_name = String::default();
// if we're using the write cache, we can't cache the hash calc results because not all accounts are in append vecs.
if (should_cache_hash_data && eligible_for_caching)
|| config.store_detailed_debug_info_on_failure
{
Expand Down Expand Up @@ -7033,30 +7029,6 @@ impl AccountsDb {

for (slot, sub_storages) in snapshot_storages.iter_range(start..end_exclusive) {
scanner.set_slot(slot);
let valid_slot = sub_storages.is_some();
if config.use_write_cache {
let ancestors = config.ancestors.as_ref().unwrap();
if let Some(slot_cache) = self.accounts_cache.slot_cache(slot) {
if valid_slot
|| ancestors.contains_key(&slot)
|| self.accounts_index.is_alive_root(slot)
{
let keys = slot_cache.get_all_pubkeys();
for key in keys {
if scanner.filter(&key) {
if let Some(cached_account) = slot_cache.get_cloned(&key) {
let mut accessor = LoadedAccountAccessor::Cached(Some(
Cow::Owned(cached_account),
));
let account = accessor.get_loaded_account().unwrap();
scanner.found_account(&account);
}
}
}
}
}
}

if let Some(sub_storages) = sub_storages {
Self::scan_multiple_account_storages_one_slot(sub_storages, &mut scanner);
}
Expand Down Expand Up @@ -7118,7 +7090,7 @@ impl AccountsDb {
config: &CalcAccountsHashConfig<'_>,
) -> Result<(Hash, u64), BankHashVerificationError> {
if !use_index {
if !config.use_write_cache && self.accounts_cache.contains_any_slots(slot) {
if self.accounts_cache.contains_any_slots(slot) {
// this indicates a race condition
inc_new_counter_info!("accounts_hash_items_in_write_cache", 1);
}
Expand Down Expand Up @@ -7181,7 +7153,6 @@ impl AccountsDb {
slot: Slot,
ancestors: &Ancestors,
expected_capitalization: Option<u64>,
can_cached_slot_be_unflushed: bool,
epoch_schedule: &EpochSchedule,
rent_collector: &RentCollector,
is_startup: bool,
Expand All @@ -7197,7 +7168,6 @@ impl AccountsDb {
use_bg_thread_pool: !is_startup,
check_hash,
ancestors: Some(ancestors),
use_write_cache: can_cached_slot_be_unflushed,
epoch_schedule,
rent_collector,
store_detailed_debug_info_on_failure: false,
Expand Down Expand Up @@ -7346,11 +7316,6 @@ impl AccountsDb {
let _guard = self.active_stats.activate(ActiveStatItem::Hash);
stats.oldest_root = storages.range().start;

assert!(
!(config.store_detailed_debug_info_on_failure && config.use_write_cache),
"cannot accurately capture all data for debugging if accounts cache is being used"
);

self.mark_old_slots_as_dirty(storages, config.epoch_schedule.slots_per_epoch, &mut stats);

let (num_hash_scan_passes, bins_per_pass) = Self::bins_per_pass(self.num_hash_scan_passes);
Expand Down Expand Up @@ -7452,7 +7417,6 @@ impl AccountsDb {
test_hash_calculation: bool,
epoch_schedule: &EpochSchedule,
rent_collector: &RentCollector,
can_cached_slot_be_unflushed: bool,
enable_rehashing: bool,
) -> Result<(), BankHashVerificationError> {
self.verify_bank_hash_and_lamports_new(
Expand All @@ -7462,7 +7426,6 @@ impl AccountsDb {
test_hash_calculation,
epoch_schedule,
rent_collector,
can_cached_slot_be_unflushed,
false,
false,
enable_rehashing,
Expand All @@ -7479,7 +7442,6 @@ impl AccountsDb {
test_hash_calculation: bool,
epoch_schedule: &EpochSchedule,
rent_collector: &RentCollector,
can_cached_slot_be_unflushed: bool,
ignore_mismatch: bool,
store_hash_raw_data_for_debug: bool,
enable_rehashing: bool,
Expand All @@ -7499,7 +7461,6 @@ impl AccountsDb {
use_bg_thread_pool: !is_startup,
check_hash,
ancestors: Some(ancestors),
use_write_cache: can_cached_slot_be_unflushed,
epoch_schedule,
rent_collector,
store_detailed_debug_info_on_failure: store_hash_raw_data_for_debug,
Expand Down Expand Up @@ -11777,7 +11738,6 @@ pub mod tests {
true,
&EpochSchedule::default(),
&RentCollector::default(),
false,
true,
)
.unwrap();
Expand Down Expand Up @@ -12109,7 +12069,6 @@ pub mod tests {
use_bg_thread_pool: false,
check_hash: false,
ancestors: None,
use_write_cache: false,
epoch_schedule: &EPOCH_SCHEDULE,
rent_collector: &RENT_COLLECTOR,
store_detailed_debug_info_on_failure: false,
Expand Down Expand Up @@ -12183,7 +12142,6 @@ pub mod tests {
true,
&EpochSchedule::default(),
&RentCollector::default(),
false,
true,
),
Ok(_)
Expand All @@ -12198,7 +12156,6 @@ pub mod tests {
true,
&EpochSchedule::default(),
&RentCollector::default(),
false,
true,
),
Err(MissingBankHash)
Expand All @@ -12222,7 +12179,6 @@ pub mod tests {
true,
&EpochSchedule::default(),
&RentCollector::default(),
false,
true,
),
Err(MismatchedBankHash)
Expand Down Expand Up @@ -12252,7 +12208,6 @@ pub mod tests {
true,
&EpochSchedule::default(),
&RentCollector::default(),
false,
true,
),
Ok(_)
Expand All @@ -12275,14 +12230,13 @@ pub mod tests {
true,
&EpochSchedule::default(),
&RentCollector::default(),
false,
true,
),
Ok(_)
);

assert_matches!(
db.verify_bank_hash_and_lamports(some_slot, &ancestors, 10, true, &EpochSchedule::default(), &RentCollector::default(), false, true,),
db.verify_bank_hash_and_lamports(some_slot, &ancestors, 10, true, &EpochSchedule::default(), &RentCollector::default(), true,),
Err(MismatchedTotalLamports(expected, actual)) if expected == 2 && actual == 10
);
}
Expand All @@ -12309,7 +12263,6 @@ pub mod tests {
true,
&EpochSchedule::default(),
&RentCollector::default(),
false,
true,
),
Ok(_)
Expand Down Expand Up @@ -12354,7 +12307,6 @@ pub mod tests {
true,
&EpochSchedule::default(),
&RentCollector::default(),
false,
true,
),
Err(MismatchedBankHash)
Expand Down Expand Up @@ -12975,7 +12927,6 @@ pub mod tests {
true,
&EpochSchedule::default(),
&RentCollector::default(),
false,
true,
)
.unwrap();
Expand All @@ -12989,7 +12940,6 @@ pub mod tests {
true,
&EpochSchedule::default(),
&RentCollector::default(),
false,
true,
)
.unwrap();
Expand Down
4 changes: 1 addition & 3 deletions runtime/src/accounts_hash.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,10 @@ pub struct CalcAccountsHashConfig<'a> {
/// verify every hash in append vec/write cache with a recalculated hash
/// this option will be removed
pub check_hash: bool,
/// 'ancestors' is used to get storages and also used if 'use_write_cache' is true to
/// get account data from the write cache
/// 'ancestors' is used to get storages
pub ancestors: Option<&'a Ancestors>,
/// does hash calc need to consider account data that exists in the write cache?
/// if so, 'ancestors' will be used for this purpose as well as storages.
pub use_write_cache: bool,
pub epoch_schedule: &'a EpochSchedule,
pub rent_collector: &'a RentCollector,
/// used for tracking down hash mismatches after the fact
Expand Down
Loading

0 comments on commit cfc124c

Please sign in to comment.