Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Commit

Permalink
Handle cleaning zero-lamport accounts
Browse files Browse the repository at this point in the history
Handle cleaning zero-lamport accounts in slots higher than the last full
snapshot slot.  This is part of the Incremental Snapshot work.

Fixes #18825
  • Loading branch information
brooksprumo committed Aug 10, 2021
1 parent f302774 commit 3af2c62
Showing 10 changed files with 491 additions and 119 deletions.
2 changes: 1 addition & 1 deletion accounts-bench/src/main.rs
Original file line number Diff line number Diff line change
@@ -102,7 +102,7 @@ fn main() {
for x in 0..iterations {
if clean {
let mut time = Measure::start("clean");
accounts.accounts_db.clean_accounts(None, false);
accounts.accounts_db.clean_accounts(None, false, None);
time.stop();
println!("{}", time);
for slot in 0..num_slots {
2 changes: 1 addition & 1 deletion ledger/src/blockstore_processor.rs
Original file line number Diff line number Diff line change
@@ -1076,7 +1076,7 @@ fn load_frozen_forks(
// Must be called after `squash()`, so that AccountsDb knows what
// the roots are for the cache flushing in exhaustively_free_unused_resource().
// This could take few secs; so update last_free later
new_root_bank.exhaustively_free_unused_resource();
new_root_bank.exhaustively_free_unused_resource(None);
last_free = Instant::now();
}

2 changes: 1 addition & 1 deletion runtime/benches/accounts.rs
Original file line number Diff line number Diff line change
@@ -176,7 +176,7 @@ fn bench_delete_dependencies(bencher: &mut Bencher) {
accounts.add_root(i);
}
bencher.iter(|| {
accounts.accounts_db.clean_accounts(None, false);
accounts.accounts_db.clean_accounts(None, false, None);
});
}

2 changes: 1 addition & 1 deletion runtime/src/accounts.rs
Original file line number Diff line number Diff line change
@@ -2078,7 +2078,7 @@ mod tests {
}
}
info!("done..cleaning..");
accounts.accounts_db.clean_accounts(None, false);
accounts.accounts_db.clean_accounts(None, false, None);
}

fn load_accounts_no_store(accounts: &Accounts, tx: Transaction) -> Vec<TransactionLoadResult> {
4 changes: 2 additions & 2 deletions runtime/src/accounts_background_service.rs
Original file line number Diff line number Diff line change
@@ -160,7 +160,7 @@ impl SnapshotRequestHandler {
// accounts that were included in the bank delta hash when the bank was frozen,
// and if we clean them here, the newly created snapshot's hash may not match
// the frozen hash.
snapshot_root_bank.clean_accounts(true, false);
snapshot_root_bank.clean_accounts(true, false, None);
clean_time.stop();

if accounts_db_caching_enabled {
@@ -399,7 +399,7 @@ impl AccountsBackgroundService {
// slots >= bank.slot()
bank.force_flush_accounts_cache();
}
bank.clean_accounts(true, false);
bank.clean_accounts(true, false, None);
last_cleaned_block_height = bank.block_height();
}
}
301 changes: 225 additions & 76 deletions runtime/src/accounts_db.rs

Large diffs are not rendered by default.

72 changes: 41 additions & 31 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
@@ -2441,7 +2441,7 @@ impl Bank {

// Should not be called outside of startup, will race with
// concurrent cleaning logic in AccountsBackgroundService
pub fn exhaustively_free_unused_resource(&self) {
pub fn exhaustively_free_unused_resource(&self, last_full_snapshot_slot: Option<Slot>) {
let mut flush = Measure::start("flush");
// Flush all the rooted accounts. Must be called after `squash()`,
// so that AccountsDb knows what the roots are.
@@ -2453,11 +2453,11 @@ impl Bank {
// accounts that were included in the bank delta hash when the bank was frozen,
// and if we clean them here, any newly created snapshot's hash for this bank
// may not match the frozen hash.
self.clean_accounts(true, false);
self.clean_accounts(true, false, last_full_snapshot_slot);
clean.stop();

let mut shrink = Measure::start("shrink");
self.shrink_all_slots(false);
self.shrink_all_slots(false, last_full_snapshot_slot);
shrink.stop();

info!(
@@ -4998,18 +4998,19 @@ impl Bank {
&self,
test_hash_calculation: bool,
accounts_db_skip_shrink: bool,
last_full_snapshot_slot: Option<Slot>,
) -> bool {
info!("cleaning..");
let mut clean_time = Measure::start("clean");
if self.slot() > 0 {
self.clean_accounts(true, true);
self.clean_accounts(true, true, last_full_snapshot_slot);
}
clean_time.stop();

let mut shrink_all_slots_time = Measure::start("shrink_all_slots");
if !accounts_db_skip_shrink && self.slot() > 0 {
info!("shrinking..");
self.shrink_all_slots(true);
self.shrink_all_slots(true, last_full_snapshot_slot);
}
shrink_all_slots_time.stop();

@@ -5285,24 +5286,33 @@ impl Bank {
.add_program(program_id, process_instruction_with_context);
}

pub fn clean_accounts(&self, skip_last: bool, is_startup: bool) {
let max_clean_slot = if skip_last {
// Don't clean the slot we're snapshotting because it may have zero-lamport
// accounts that were included in the bank delta hash when the bank was frozen,
// and if we clean them here, any newly created snapshot's hash for this bank
// may not match the frozen hash.
Some(self.slot().saturating_sub(1))
} else {
None
};
pub fn clean_accounts(
&self,
skip_last: bool,
is_startup: bool,
last_full_snapshot_slot: Option<Slot>,
) {
// Don't clean the slot we're snapshotting because it may have zero-lamport
// accounts that were included in the bank delta hash when the bank was frozen,
// and if we clean them here, any newly created snapshot's hash for this bank
// may not match the frozen hash.
//
// So when we're snapshotting, set `skip_last` to true so the highest slot to clean is
// lowered by one.
let highest_slot_to_clean = skip_last.then(|| self.slot().saturating_sub(1));

self.rc.accounts.accounts_db.clean_accounts(
highest_slot_to_clean,
is_startup,
last_full_snapshot_slot,
);
}

pub fn shrink_all_slots(&self, is_startup: bool, last_full_snapshot_slot: Option<Slot>) {
self.rc
.accounts
.accounts_db
.clean_accounts(max_clean_slot, is_startup);
}

pub fn shrink_all_slots(&self, is_startup: bool) {
self.rc.accounts.accounts_db.shrink_all_slots(is_startup);
.shrink_all_slots(is_startup, last_full_snapshot_slot);
}

pub fn print_accounts_stats(&self) {
@@ -7883,7 +7893,7 @@ pub(crate) mod tests {
bank.squash();
bank.force_flush_accounts_cache();
let hash = bank.update_accounts_hash();
bank.clean_accounts(false, false);
bank.clean_accounts(false, false, None);
assert_eq!(bank.update_accounts_hash(), hash);

let bank0 = Arc::new(new_from_parent(&bank));
@@ -7903,14 +7913,14 @@ pub(crate) mod tests {

info!("bank0 purge");
let hash = bank0.update_accounts_hash();
bank0.clean_accounts(false, false);
bank0.clean_accounts(false, false, None);
assert_eq!(bank0.update_accounts_hash(), hash);

assert_eq!(bank0.get_account(&keypair.pubkey()).unwrap().lamports(), 10);
assert_eq!(bank1.get_account(&keypair.pubkey()), None);

info!("bank1 purge");
bank1.clean_accounts(false, false);
bank1.clean_accounts(false, false, None);

assert_eq!(bank0.get_account(&keypair.pubkey()).unwrap().lamports(), 10);
assert_eq!(bank1.get_account(&keypair.pubkey()), None);
@@ -7931,7 +7941,7 @@ pub(crate) mod tests {
assert_eq!(bank0.get_account(&keypair.pubkey()), None);
assert_eq!(bank1.get_account(&keypair.pubkey()), None);
bank1.force_flush_accounts_cache();
bank1.clean_accounts(false, false);
bank1.clean_accounts(false, false, None);

assert!(bank1.verify_bank_hash(true));
}
@@ -8796,11 +8806,11 @@ pub(crate) mod tests {
bank.transfer(1_000, &mint_keypair, &pubkey).unwrap();
bank.freeze();
bank.update_accounts_hash();
assert!(bank.verify_snapshot_bank(true, false));
assert!(bank.verify_snapshot_bank(true, false, None));

// tamper the bank after freeze!
bank.increment_signature_count(1);
assert!(!bank.verify_snapshot_bank(true, false));
assert!(!bank.verify_snapshot_bank(true, false, None));
}

// Test that two bank forks with the same accounts should not hash to the same value.
@@ -11230,7 +11240,7 @@ pub(crate) mod tests {

// Clean accounts, which should add earlier slots to the shrink
// candidate set
bank2.clean_accounts(false, false);
bank2.clean_accounts(false, false, None);

// Slots 0 and 1 should be candidates for shrinking, but slot 2
// shouldn't because none of its accounts are outdated by a later
@@ -11286,7 +11296,7 @@ pub(crate) mod tests {
goto_end_of_slot(Arc::<Bank>::get_mut(&mut bank).unwrap());

bank.squash();
bank.clean_accounts(false, false);
bank.clean_accounts(false, false, None);
let force_to_return_alive_account = 0;
assert_eq!(
bank.process_stale_slot_with_budget(22, force_to_return_alive_account),
@@ -12914,7 +12924,7 @@ pub(crate) mod tests {
current_major_fork_bank.squash();
// Try to get cache flush/clean to overlap with the scan
current_major_fork_bank.force_flush_accounts_cache();
current_major_fork_bank.clean_accounts(false, false);
current_major_fork_bank.clean_accounts(false, false, None);
// Move purge here so that Bank::drop()->purge_slots() doesn't race
// with clean. Simulates the call from AccountsBackgroundService
let is_abs_service = true;
@@ -12964,7 +12974,7 @@ pub(crate) mod tests {
current_bank.squash();
if current_bank.slot() % 2 == 0 {
current_bank.force_flush_accounts_cache();
current_bank.clean_accounts(true, false);
current_bank.clean_accounts(true, false, None);
}
prev_bank = current_bank.clone();
current_bank = Arc::new(Bank::new_from_parent(
@@ -13929,7 +13939,7 @@ pub(crate) mod tests {
bank2.squash();

drop(bank1);
bank2.clean_accounts(false, false);
bank2.clean_accounts(false, false, None);

let expected_ref_count_for_cleaned_up_keys = 0;
let expected_ref_count_for_keys_in_both_slot1_and_slot2 = 1;
Loading

0 comments on commit 3af2c62

Please sign in to comment.