Skip to content

Commit

Permalink
feature: on accounts hash calculation, do not try to rehash accounts s…
Browse files Browse the repository at this point in the history 
…olana-labs#28934
  • Loading branch information
@jeffwashington
jeffwashington committed Nov 23, 2022
1 parent 4d8b42c commit 02a059a
Show file tree
Hide file tree
Showing 7 changed files with 1,408 additions and 5 deletions.
4 changes: 4 additions & 0 deletions runtime/src/accounts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,7 @@ impl Accounts {
debug_verify: bool,
epoch_schedule: &EpochSchedule,
rent_collector: &RentCollector,
disable_rehash_for_rent_epoch: bool,
) -> u64 {
let use_index = false;
let is_startup = true;
Expand All @@ -828,6 +829,7 @@ impl Accounts {
epoch_schedule,
rent_collector,
is_startup,
disable_rehash_for_rent_epoch,
)
.1
}
Expand All @@ -847,6 +849,7 @@ impl Accounts {
ignore_mismatch: bool,
store_detailed_debug_info: bool,
use_bg_thread_pool: bool,
disable_rehash_for_rent_epoch: bool,
) -> bool {
if let Err(err) = self.accounts_db.verify_bank_hash_and_lamports_new(
slot,
Expand All @@ -859,6 +862,7 @@ impl Accounts {
ignore_mismatch,
store_detailed_debug_info,
use_bg_thread_pool,
disable_rehash_for_rent_epoch,
) {
warn!("verify_bank_hash failed: {:?}, slot: {}", err, slot);
false
Expand Down
1 change: 1 addition & 0 deletions runtime/src/accounts_background_service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ impl SnapshotRequestHandler {
rent_collector: snapshot_root_bank.rent_collector(),
store_detailed_debug_info_on_failure: false,
full_snapshot: None,
disable_rehash_for_rent_epoch: snapshot_root_bank.feature_set.is_active(&solana_sdk::feature_set::disable_rehash_for_rent_epoch::id()),
},
).unwrap();
assert_eq!(previous_hash, this_hash);
Expand Down
84 changes: 79 additions & 5 deletions runtime/src/accounts_db.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ use {
bank::Rewrites,
cache_hash_data::CacheHashData,
contains::Contains,
expected_rent_collection::{ExpectedRentCollection, SlotInfoInEpoch},
pubkey_bins::PubkeyBinCalculator24,
read_only_accounts_cache::ReadOnlyAccountsCache,
rent_collector::RentCollector,
Expand Down Expand Up @@ -113,6 +114,8 @@ pub const DEFAULT_NUM_DIRS: u32 = 4;
pub const PUBKEY_BINS_FOR_CALCULATING_HASHES: usize = 65536;
pub const NUM_SCAN_PASSES_DEFAULT: usize = 2;

pub const TEST_DISABLE_REHASH_FOR_RENT_EPOCH: bool = true;

// Without chunks, we end up with 1 output vec for each outer snapshot storage.
// This results in too many vectors to be efficient.
// Chunks when scanning storages to calculate hashes.
Expand Down Expand Up @@ -1792,22 +1795,26 @@ trait AppendVecScan: Send + Sync + Clone {
/// These would have been captured in a fn from within the scan function.
/// Some of these are constant across all pubkeys, some are constant across a slot.
/// Some could be unique per pubkey.
struct ScanState<'a> {
struct ScanState<'a, T: Fn(Slot) -> Option<Slot> + Sync + Send + Clone> {
/// slot we're currently scanning
current_slot: Slot,
/// accumulated results
accum: BinnedHashData,
/// max slot (inclusive) that we're calculating accounts hash on
max_slot_info: SlotInfoInEpoch,
bin_calculator: &'a PubkeyBinCalculator24,
bin_range: &'a Range<usize>,
config: &'a CalcAccountsHashConfig<'a>,
mismatch_found: Arc<AtomicU64>,
stats: &'a crate::accounts_hash::HashStats,
find_unskipped_slot: &'a T,
filler_account_suffix: Option<&'a Pubkey>,
range: usize,
sort_time: Arc<AtomicU64>,
pubkey_to_bin_index: usize,
}

impl<'a> AppendVecScan for ScanState<'a> {
impl<'a, T: Fn(Slot) -> Option<Slot> + Sync + Send + Clone> AppendVecScan for ScanState<'a, T> {
fn set_slot(&mut self, slot: Slot) {
self.current_slot = slot;
}
Expand Down Expand Up @@ -1836,6 +1843,24 @@ impl<'a> AppendVecScan for ScanState<'a> {
};

let loaded_hash = loaded_account.loaded_hash();
let new_hash = (!self.config.disable_rehash_for_rent_epoch)
.then(|| {
ExpectedRentCollection::maybe_rehash_skipped_rewrite(
loaded_account,
&loaded_hash,
pubkey,
self.current_slot,
self.config.epoch_schedule,
self.config.rent_collector,
self.stats,
&self.max_slot_info,
self.find_unskipped_slot,
self.filler_account_suffix,
)
})
.flatten();
let loaded_hash = new_hash.unwrap_or(loaded_hash);

let source_item = CalculateHashIntermediate::new(loaded_hash, balance, *pubkey);

if self.config.check_hash
Expand Down Expand Up @@ -6236,6 +6261,8 @@ impl AccountsDb {
let total_lamports = Mutex::<u64>::new(0);
let stats = HashStats::default();

let max_slot_info = SlotInfoInEpoch::new(max_slot, config.epoch_schedule);

let get_hashes = || {
keys.par_chunks(chunks)
.map(|pubkeys| {
Expand Down Expand Up @@ -6268,7 +6295,23 @@ impl AccountsDb {
.get_loaded_account()
.and_then(
|loaded_account| {
let find_unskipped_slot = |slot: Slot| {
self.find_unskipped_slot(slot, config.ancestors)
};
let loaded_hash = loaded_account.loaded_hash();
let new_hash = (!config.disable_rehash_for_rent_epoch).then(||ExpectedRentCollection::maybe_rehash_skipped_rewrite(
&loaded_account,
&loaded_hash,
pubkey,
*slot,
config.epoch_schedule,
config.rent_collector,
&stats,
&max_slot_info,
find_unskipped_slot,
self.filler_account_suffix.as_ref(),
)).flatten();
let loaded_hash = new_hash.unwrap_or(loaded_hash);
let balance = loaded_account.lamports();
if config.check_hash && !self.is_filler_account(pubkey) { // this will not be supported anymore
let computed_hash =
Expand Down Expand Up @@ -6353,6 +6396,7 @@ impl AccountsDb {
ancestors: &Ancestors,
epoch_schedule: &EpochSchedule,
rent_collector: &RentCollector,
disable_rehash_for_rent_epoch: bool,
) -> (Hash, u64) {
self.update_accounts_hash_with_index_option(
true,
Expand All @@ -6364,6 +6408,7 @@ impl AccountsDb {
epoch_schedule,
rent_collector,
false,
disable_rehash_for_rent_epoch,
)
}

Expand All @@ -6379,6 +6424,7 @@ impl AccountsDb {
&EpochSchedule::default(),
&RentCollector::default(),
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
)
}

Expand Down Expand Up @@ -6803,6 +6849,7 @@ impl AccountsDb {
epoch_schedule: &EpochSchedule,
rent_collector: &RentCollector,
is_startup: bool,
disable_rehash_for_rent_epoch: bool,
) -> (Hash, u64) {
let check_hash = false;
let (hash, total_lamports) = self
Expand All @@ -6819,6 +6866,7 @@ impl AccountsDb {
rent_collector,
store_detailed_debug_info_on_failure: false,
full_snapshot: None,
disable_rehash_for_rent_epoch,
},
expected_capitalization,
)
Expand Down Expand Up @@ -6853,15 +6901,22 @@ impl AccountsDb {
let range = bin_range.end - bin_range.start;
let sort_time = Arc::new(AtomicU64::new(0));

let find_unskipped_slot = |slot: Slot| self.find_unskipped_slot(slot, config.ancestors);

let max_slot_info =
SlotInfoInEpoch::new(storage.max_slot_inclusive(), config.epoch_schedule);
let scanner = ScanState {
current_slot: Slot::default(),
accum: BinnedHashData::default(),
bin_calculator: &bin_calculator,
config,
mismatch_found: mismatch_found.clone(),
max_slot_info,
find_unskipped_slot: &find_unskipped_slot,
filler_account_suffix,
range,
bin_range,
stats,
sort_time: sort_time.clone(),
pubkey_to_bin_index: 0,
};
Expand Down Expand Up @@ -7052,6 +7107,7 @@ impl AccountsDb {
rent_collector: &RentCollector,
can_cached_slot_be_unflushed: bool,
use_bg_thread_pool: bool,
disable_rehash_for_rent_epoch: bool,
) -> Result<(), BankHashVerificationError> {
self.verify_bank_hash_and_lamports_new(
slot,
Expand All @@ -7064,6 +7120,7 @@ impl AccountsDb {
false,
false,
use_bg_thread_pool,
disable_rehash_for_rent_epoch,
)
}

Expand All @@ -7081,6 +7138,7 @@ impl AccountsDb {
ignore_mismatch: bool,
store_hash_raw_data_for_debug: bool,
use_bg_thread_pool: bool,
disable_rehash_for_rent_epoch: bool,
) -> Result<(), BankHashVerificationError> {
use BankHashVerificationError::*;

Expand All @@ -7100,6 +7158,7 @@ impl AccountsDb {
rent_collector,
store_detailed_debug_info_on_failure: store_hash_raw_data_for_debug,
full_snapshot: None,
disable_rehash_for_rent_epoch,
},
None,
)?;
Expand Down Expand Up @@ -11083,13 +11142,15 @@ pub mod tests {
latest_slot,
&ancestors,
&EpochSchedule::default(),
&RentCollector::default()
&RentCollector::default(),
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
),
accounts.update_accounts_hash(
latest_slot,
&ancestors,
&EpochSchedule::default(),
&RentCollector::default()
&RentCollector::default(),
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
)
);
}
Expand Down Expand Up @@ -11373,6 +11434,7 @@ pub mod tests {
&Ancestors::default(),
&EpochSchedule::default(),
&RentCollector::default(),
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
);

let accounts = f(accounts, current_slot);
Expand All @@ -11394,6 +11456,7 @@ pub mod tests {
&RentCollector::default(),
false,
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
)
.unwrap();
}
Expand Down Expand Up @@ -11729,6 +11792,7 @@ pub mod tests {
rent_collector: &RENT_COLLECTOR,
store_detailed_debug_info_on_failure: false,
full_snapshot: None,
disable_rehash_for_rent_epoch: TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
}
}
}
Expand Down Expand Up @@ -11799,6 +11863,7 @@ pub mod tests {
&RentCollector::default(),
false,
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
),
Ok(_)
);
Expand All @@ -11814,6 +11879,7 @@ pub mod tests {
&RentCollector::default(),
false,
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
),
Err(MissingBankHash)
);
Expand All @@ -11838,6 +11904,7 @@ pub mod tests {
&RentCollector::default(),
false,
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
),
Err(MismatchedBankHash)
);
Expand Down Expand Up @@ -11868,6 +11935,7 @@ pub mod tests {
&RentCollector::default(),
false,
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
),
Ok(_)
);
Expand All @@ -11891,12 +11959,13 @@ pub mod tests {
&RentCollector::default(),
false,
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
),
Ok(_)
);

assert_matches!(
db.verify_bank_hash_and_lamports(some_slot, &ancestors, 10, true, &EpochSchedule::default(), &RentCollector::default(), false, false),
db.verify_bank_hash_and_lamports(some_slot, &ancestors, 10, true, &EpochSchedule::default(), &RentCollector::default(), false, false, TEST_DISABLE_REHASH_FOR_RENT_EPOCH),
Err(MismatchedTotalLamports(expected, actual)) if expected == 2 && actual == 10
);
}
Expand Down Expand Up @@ -11925,6 +11994,7 @@ pub mod tests {
&RentCollector::default(),
false,
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
),
Ok(_)
);
Expand Down Expand Up @@ -11970,6 +12040,7 @@ pub mod tests {
&RentCollector::default(),
false,
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
),
Err(MismatchedBankHash)
);
Expand Down Expand Up @@ -12579,6 +12650,7 @@ pub mod tests {
&no_ancestors,
&EpochSchedule::default(),
&RentCollector::default(),
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
);
accounts
.verify_bank_hash_and_lamports(
Expand All @@ -12590,6 +12662,7 @@ pub mod tests {
&RentCollector::default(),
false,
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
)
.unwrap();

Expand All @@ -12604,6 +12677,7 @@ pub mod tests {
&RentCollector::default(),
false,
false,
TEST_DISABLE_REHASH_FOR_RENT_EPOCH,
)
.unwrap();

Expand Down
2 changes: 2 additions & 0 deletions runtime/src/accounts_hash.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub struct CalcAccountsHashConfig<'a> {
pub store_detailed_debug_info_on_failure: bool,
/// `Some` if this is an incremental snapshot which only hashes slots since the base full snapshot
pub full_snapshot: Option<FullSnapshotAccountsHashInfo>,
/// temporarily here for feature activation #28934
pub disable_rehash_for_rent_epoch: bool,
}

impl<'a> CalcAccountsHashConfig<'a> {
Expand Down
Loading

0 comments on commit 02a059a

Please sign in to comment.