remove skip rewrites from bank (#29182)

This commit is contained in:
Jeff Washington (jwash) 2022-12-10 11:27:15 -06:00 committed by GitHub
parent 5f74fb9c87
commit db2cc53967
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 10 additions and 180 deletions

View File

@ -13,7 +13,7 @@ use {
accounts_update_notifier_interface::AccountsUpdateNotifier,
ancestors::Ancestors,
bank::{
Bank, NonceFull, NonceInfo, RentDebits, Rewrites, TransactionCheckResult,
Bank, NonceFull, NonceInfo, RentDebits, TransactionCheckResult,
TransactionExecutionResult,
},
blockhash_queue::BlockhashQueue,
@ -1209,10 +1209,8 @@ impl Accounts {
}
}
pub fn bank_hash_info_at(&self, slot: Slot, rewrites: &Rewrites) -> BankHashInfo {
let accounts_delta_hash = self
.accounts_db
.get_accounts_delta_hash_with_rewrites(slot, rewrites);
pub fn bank_hash_info_at(&self, slot: Slot) -> BankHashInfo {
let accounts_delta_hash = self.accounts_db.get_accounts_delta_hash(slot);
let bank_hashes = self.accounts_db.bank_hashes.read().unwrap();
let mut bank_hash_info = bank_hashes
.get(&slot)
@ -2629,7 +2627,7 @@ mod tests {
AccountSecondaryIndexes::default(),
AccountShrinkThreshold::default(),
);
accounts.bank_hash_info_at(1, &Rewrites::default());
accounts.bank_hash_info_at(1);
}
#[test]

View File

@ -45,7 +45,6 @@ use {
StoredAccountMeta, StoredMeta, StoredMetaWriteVersion, APPEND_VEC_MMAPPED_FILES_OPEN,
STORE_META_OVERHEAD,
},
bank::Rewrites,
cache_hash_data::{CacheHashData, CacheHashDataFile},
contains::Contains,
epoch_accounts_hash::EpochAccountsHashManager,
@ -7733,10 +7732,6 @@ impl AccountsDb {
}
}
pub fn get_accounts_delta_hash(&self, slot: Slot) -> Hash {
self.get_accounts_delta_hash_with_rewrites(slot, &Rewrites::default())
}
/// helper to return
/// 1. pubkey, hash pairs for the slot
/// 2. us spent scanning
@ -7781,11 +7776,7 @@ impl AccountsDb {
(hashes, scan.as_us(), accumulate)
}
pub fn get_accounts_delta_hash_with_rewrites(
&self,
slot: Slot,
skipped_rewrites: &Rewrites,
) -> Hash {
pub fn get_accounts_delta_hash(&self, slot: Slot) -> Hash {
let (mut hashes, scan_us, mut accumulate) = self.get_pubkey_hash_for_slot(slot);
let dirty_keys = hashes.iter().map(|(pubkey, _hash)| *pubkey).collect();
@ -7794,8 +7785,6 @@ impl AccountsDb {
hashes.retain(|(pubkey, _hash)| !self.is_filler_account(pubkey));
}
self.extend_hashes_with_skipped_rewrites(&mut hashes, skipped_rewrites);
let ret = AccountsHasher::accumulate_account_hashes(hashes);
accumulate.stop();
let mut uncleaned_time = Measure::start("uncleaned_index");
@ -7815,38 +7804,6 @@ impl AccountsDb {
ret
}
/// add all items from 'skipped_rewrites' to 'hashes' where the pubkey doesn't already exist in 'hashes'
fn extend_hashes_with_skipped_rewrites(
&self,
hashes: &mut Vec<(Pubkey, Hash)>,
skipped_rewrites: &Rewrites,
) {
let mut skipped_rewrites = skipped_rewrites.read().unwrap().clone();
if skipped_rewrites.is_empty() {
// if there are no skipped rewrites, then there is nothing futher to do
return;
}
hashes.iter().for_each(|(key, _)| {
skipped_rewrites.remove(key);
});
if self.filler_accounts_enabled() {
// simulate the time we would normally spend hashing the filler accounts
// this is an over approximation but at least takes a stab at simulating what the validator would spend time doing
let _ = AccountsHasher::accumulate_account_hashes(
skipped_rewrites
.iter()
.map(|(k, v)| (*k, *v))
.collect::<Vec<_>>(),
);
// filler accounts do not get their updated hash values hashed into the delta hash
skipped_rewrites.retain(|key, _| !self.is_filler_account(key));
}
hashes.extend(skipped_rewrites.into_iter());
}
fn update_index<'a, T: ReadableAccount + Sync>(
&self,
infos: Vec<AccountInfo>,
@ -16548,28 +16505,6 @@ pub mod tests {
}
}
#[test]
fn test_extend_hashes_with_skipped_rewrites() {
let db = AccountsDb::new_single_for_tests();
let mut hashes = Vec::default();
let rewrites = Rewrites::default();
db.extend_hashes_with_skipped_rewrites(&mut hashes, &rewrites);
assert!(hashes.is_empty());
let pubkey = Pubkey::new(&[1; 32]);
let hash = Hash::new(&[2; 32]);
rewrites.write().unwrap().insert(pubkey, hash);
db.extend_hashes_with_skipped_rewrites(&mut hashes, &rewrites);
assert_eq!(hashes, vec![(pubkey, hash)]);
// pubkey is already in hashes, will not be added a second time
db.extend_hashes_with_skipped_rewrites(&mut hashes, &rewrites);
assert_eq!(hashes, vec![(pubkey, hash)]);
let pubkey2 = Pubkey::new(&[2; 32]);
let hash2 = Hash::new(&[3; 32]);
rewrites.write().unwrap().insert(pubkey2, hash2);
db.extend_hashes_with_skipped_rewrites(&mut hashes, &rewrites);
assert_eq!(hashes, vec![(pubkey, hash), (pubkey2, hash2)]);
}
#[test]
fn test_calc_alive_ancient_historical_roots() {
let db = AccountsDb::new_single_for_tests();

View File

@ -210,8 +210,6 @@ pub const SECONDS_PER_YEAR: f64 = 365.25 * 24.0 * 60.0 * 60.0;
pub const MAX_LEADER_SCHEDULE_STAKES: Epoch = 5;
pub type Rewrites = RwLock<HashMap<Pubkey, Hash>>;
#[derive(Default)]
struct RentMetrics {
hold_range_us: AtomicU64,
@ -862,7 +860,6 @@ impl PartialEq for Bank {
freeze_started: _,
vote_only_bank: _,
cost_tracker: _,
rewrites_skipped_this_slot: _,
sysvar_cache: _,
accounts_data_size_initial: _,
accounts_data_size_delta_on_chain: _,
@ -1110,9 +1107,6 @@ pub struct Bank {
sysvar_cache: RwLock<SysvarCache>,
/// (Pubkey, account Hash) for each account that would have been rewritten in rent collection for this slot
pub rewrites_skipped_this_slot: Rewrites,
/// The initial accounts data size at the start of this Bank, before processing any transactions/etc
accounts_data_size_initial: u64,
/// The change to accounts data size in this Bank, due on-chain events (i.e. transactions)
@ -1270,7 +1264,6 @@ impl Bank {
fn default_with_accounts(accounts: Accounts) -> Self {
let mut bank = Self {
incremental_snapshot_persistence: None,
rewrites_skipped_this_slot: Rewrites::default(),
rc: BankRc::new(accounts, Slot::default()),
status_cache: Arc::<RwLock<BankStatusCache>>::default(),
blockhash_queue: RwLock::<BlockhashQueue>::default(),
@ -1574,7 +1567,6 @@ impl Bank {
let accounts_data_size_initial = parent.load_accounts_data_size();
let mut new = Bank {
incremental_snapshot_persistence: None,
rewrites_skipped_this_slot: Rewrites::default(),
rc,
status_cache,
slot,
@ -1958,7 +1950,6 @@ impl Bank {
let feature_set = new();
let mut bank = Self {
incremental_snapshot_persistence: fields.incremental_snapshot_persistence,
rewrites_skipped_this_slot: Rewrites::default(),
rc: bank_rc,
status_cache: new(),
blockhash_queue: RwLock::new(fields.blockhash_queue),
@ -5299,11 +5290,6 @@ impl Bank {
"collect_rent_eagerly",
("accounts", rent_metrics.count.load(Relaxed), i64),
("partitions", count, i64),
(
"skipped_rewrites",
self.rewrites_skipped_this_slot.read().unwrap().len(),
i64
),
("total_time_us", measure.as_us(), i64),
(
"hold_range_us",
@ -5431,7 +5417,6 @@ impl Bank {
CollectRentFromAccountsInfo {
rent_collected_info: total_rent_collected_info,
rent_rewards: rent_debits.into_unordered_rewards_iter().collect(),
rewrites_skipped,
time_collecting_rent_us,
time_hashing_skipped_rewrites_us,
time_storing_accounts_us,
@ -5572,7 +5557,6 @@ impl Bank {
.write()
.unwrap()
.append(&mut results.rent_rewards);
self.remember_skipped_rewrites(results.rewrites_skipped);
metrics
.load_us
@ -5590,16 +5574,6 @@ impl Bank {
});
}
// put 'rewrites_skipped' into 'self.rewrites_skipped_this_slot'
fn remember_skipped_rewrites(&self, rewrites_skipped: Vec<(Pubkey, Hash)>) {
if !rewrites_skipped.is_empty() {
let mut rewrites_skipped_this_slot = self.rewrites_skipped_this_slot.write().unwrap();
rewrites_skipped.into_iter().for_each(|(pubkey, hash)| {
rewrites_skipped_this_slot.insert(pubkey, hash);
});
}
}
/// return true iff storing this account is just a rewrite and can be skipped
fn skip_rewrite(rent_amount: u64, account: &AccountSharedData) -> bool {
// if rent was != 0
@ -6720,10 +6694,7 @@ impl Bank {
/// of the delta of the ledger since the last vote and up to now
fn hash_internal_state(&self) -> Hash {
// If there are no accounts, return the hash of the previous state and the latest blockhash
let bank_hash_info = self
.rc
.accounts
.bank_hash_info_at(self.slot(), &self.rewrites_skipped_this_slot);
let bank_hash_info = self.rc.accounts.bank_hash_info_at(self.slot());
let mut signature_count_buf = [0u8; 8];
LittleEndian::write_u64(&mut signature_count_buf[..], self.signature_count());
@ -7877,7 +7848,6 @@ enum ApplyFeatureActivationsCaller {
struct CollectRentFromAccountsInfo {
rent_collected_info: CollectedInfo,
rent_rewards: Vec<(Pubkey, RewardInfo)>,
rewrites_skipped: Vec<(Pubkey, Hash)>,
time_collecting_rent_us: u64,
time_hashing_skipped_rewrites_us: u64,
time_storing_accounts_us: u64,
@ -7891,7 +7861,6 @@ struct CollectRentInPartitionInfo {
rent_collected: u64,
accounts_data_size_reclaimed: u64,
rent_rewards: Vec<(Pubkey, RewardInfo)>,
rewrites_skipped: Vec<(Pubkey, Hash)>,
time_loading_accounts_us: u64,
time_collecting_rent_us: u64,
time_hashing_skipped_rewrites_us: u64,
@ -7908,7 +7877,6 @@ impl CollectRentInPartitionInfo {
rent_collected: info.rent_collected_info.rent_amount,
accounts_data_size_reclaimed: info.rent_collected_info.account_data_len_reclaimed,
rent_rewards: info.rent_rewards,
rewrites_skipped: info.rewrites_skipped,
time_loading_accounts_us: time_loading_accounts.as_micros() as u64,
time_collecting_rent_us: info.time_collecting_rent_us,
time_hashing_skipped_rewrites_us: info.time_hashing_skipped_rewrites_us,
@ -7929,7 +7897,6 @@ impl CollectRentInPartitionInfo {
.accounts_data_size_reclaimed
.saturating_add(rhs.accounts_data_size_reclaimed),
rent_rewards: [lhs.rent_rewards, rhs.rent_rewards].concat(),
rewrites_skipped: [lhs.rewrites_skipped, rhs.rewrites_skipped].concat(),
time_loading_accounts_us: lhs
.time_loading_accounts_us
.saturating_add(rhs.time_loading_accounts_us),
@ -10024,25 +9991,7 @@ pub(crate) mod tests {
);
assert_eq!(bank.collected_rent.load(Relaxed), 0);
assert!(bank.rewrites_skipped_this_slot.read().unwrap().is_empty());
bank.collect_rent_in_partition((0, 0, 1), true, &RentMetrics::default());
{
let rewrites_skipped = bank.rewrites_skipped_this_slot.read().unwrap();
// `rewrites_skipped.len()` is the number of non-rent paying accounts in the slot.
// 'collect_rent_in_partition' fills 'rewrites_skipped_this_slot' with rewrites that
// were skipped during rent collection but should still be considered in the slot's
// bank hash. If the slot is also written in the append vec, then the bank hash calc
// code ignores the contents of this list. This assert is confirming that the expected #
// of accounts were included in 'rewrites_skipped' by the call to
// 'collect_rent_in_partition(..., true)' above.
assert_eq!(rewrites_skipped.len(), 1);
// should not have skipped 'rent_exempt_pubkey'
// Once preserve_rent_epoch_for_rent_exempt_accounts is activated,
// rewrite-skip is irrelevant to rent-exempt accounts.
assert!(!rewrites_skipped.contains_key(&rent_exempt_pubkey));
// should NOT have skipped 'rent_due_pubkey'
assert!(!rewrites_skipped.contains_key(&rent_due_pubkey));
}
assert_eq!(bank.collected_rent.load(Relaxed), 0);
bank.collect_rent_in_partition((0, 0, 1), false, &RentMetrics::default()); // all range
@ -10114,13 +10063,12 @@ pub(crate) mod tests {
let just_rewrites = true;
// loaded from previous slot, so we skip rent collection on it
let result = later_bank.collect_rent_from_accounts(
let _result = later_bank.collect_rent_from_accounts(
vec![(zero_lamport_pubkey, account, later_slot - 1)],
just_rewrites,
None,
PartitionIndex::default(),
);
assert!(result.rewrites_skipped[0].0 == zero_lamport_pubkey);
}
#[test]
@ -19611,53 +19559,6 @@ pub(crate) mod tests {
}
}
#[test]
fn test_remember_skipped_rewrites() {
let GenesisConfigInfo {
mut genesis_config, ..
} = create_genesis_config_with_leader(1_000_000_000, &Pubkey::new_unique(), 42);
genesis_config.rent = Rent::default();
activate_all_features(&mut genesis_config);
let bank = Bank::new_for_tests(&genesis_config);
assert!(bank.rewrites_skipped_this_slot.read().unwrap().is_empty());
bank.remember_skipped_rewrites(Vec::default());
assert!(bank.rewrites_skipped_this_slot.read().unwrap().is_empty());
// bank's map is initially empty
let mut test = vec![(Pubkey::new(&[4; 32]), Hash::new(&[5; 32]))];
bank.remember_skipped_rewrites(test.clone());
assert_eq!(
*bank.rewrites_skipped_this_slot.read().unwrap(),
test.clone().into_iter().collect()
);
// now there is already some stuff in the bank's map
test.push((Pubkey::new(&[6; 32]), Hash::new(&[7; 32])));
bank.remember_skipped_rewrites(test[1..].to_vec());
assert_eq!(
*bank.rewrites_skipped_this_slot.read().unwrap(),
test.clone().into_iter().collect()
);
// all contents are already in map
bank.remember_skipped_rewrites(test[1..].to_vec());
assert_eq!(
*bank.rewrites_skipped_this_slot.read().unwrap(),
test.clone().into_iter().collect()
);
// all contents are already in map, but we're changing hash values
test[0].1 = Hash::new(&[8; 32]);
test[1].1 = Hash::new(&[9; 32]);
bank.remember_skipped_rewrites(test.to_vec());
assert_eq!(
*bank.rewrites_skipped_this_slot.read().unwrap(),
test.into_iter().collect()
);
}
#[test]
fn test_inner_instructions_list_from_instruction_trace() {
let instruction_trace = [1, 2, 1, 1, 2, 3, 2];

View File

@ -7,7 +7,7 @@ use {
accounts_db::{get_temp_accounts_paths, AccountShrinkThreshold, AccountStorageMap},
accounts_hash::AccountsHash,
append_vec::AppendVec,
bank::{Bank, BankTestConfig, Rewrites},
bank::{Bank, BankTestConfig},
epoch_accounts_hash,
genesis_utils::{self, activate_all_features, activate_feature},
snapshot_utils::ArchiveFormat,
@ -209,12 +209,8 @@ fn test_accounts_serialize_style(serde_style: SerdeStyle) {
);
check_accounts(&daccounts, &pubkeys, 100);
assert_eq!(
accounts
.bank_hash_info_at(0, &Rewrites::default())
.accounts_delta_hash,
daccounts
.bank_hash_info_at(0, &Rewrites::default())
.accounts_delta_hash
accounts.bank_hash_info_at(0).accounts_delta_hash,
daccounts.bank_hash_info_at(0).accounts_delta_hash
);
}