Clean unrooted dropped banks (#16580) (#16911)

In a scenario where a bank is unrooted and dropped, any keys that exist
_only_ in that bank are now cleaned up.

This work was originally based on PR #15106.
This commit is contained in:
Brooks Prumo 2021-04-30 15:34:38 -05:00 committed by GitHub
parent 1594a7f11a
commit 17e6bd579f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 132 additions and 47 deletions

View File

@ -1289,13 +1289,13 @@ impl AccountsDb {
.expect("Cluster type must be set at initialization")
}
// Reclaim older states of rooted accounts for AccountsDb bloat mitigation
fn clean_old_rooted_accounts(
/// Reclaim older states of accounts older than max_clean_root for AccountsDb bloat mitigation
fn clean_accounts_older_than_root(
&self,
purges_in_root: Vec<Pubkey>,
purges: Vec<Pubkey>,
max_clean_root: Option<Slot>,
) -> ReclaimResult {
if purges_in_root.is_empty() {
if purges.is_empty() {
return (HashMap::new(), HashMap::new());
}
// This number isn't carefully chosen; just guessed randomly such that
@ -1303,21 +1303,20 @@ impl AccountsDb {
const INDEX_CLEAN_BULK_COUNT: usize = 4096;
let mut clean_rooted = Measure::start("clean_old_root-ms");
let reclaim_vecs =
purges_in_root
.par_chunks(INDEX_CLEAN_BULK_COUNT)
.map(|pubkeys: &[Pubkey]| {
let mut reclaims = Vec::new();
for pubkey in pubkeys {
self.accounts_index.clean_rooted_entries(
&pubkey,
&mut reclaims,
max_clean_root,
&self.account_indexes,
);
}
reclaims
});
let reclaim_vecs = purges
.par_chunks(INDEX_CLEAN_BULK_COUNT)
.map(|pubkeys: &[Pubkey]| {
let mut reclaims = Vec::new();
for pubkey in pubkeys {
self.accounts_index.clean_rooted_entries(
&pubkey,
&mut reclaims,
max_clean_root,
&self.account_indexes,
);
}
reclaims
});
let reclaims: Vec<_> = reclaim_vecs.flatten().collect();
clean_rooted.stop();
inc_new_counter_info!("clean-old-root-par-clean-ms", clean_rooted.as_ms() as usize);
@ -1585,20 +1584,20 @@ impl AccountsDb {
let total_keys_count = pubkeys.len();
let mut accounts_scan = Measure::start("accounts_scan");
// parallel scan the index.
let (mut purges, purges_in_root) = {
let (mut purges_zero_lamports, purges_old_accounts) = {
self.thread_pool_clean.install(|| {
pubkeys
.par_chunks(4096)
.map(|pubkeys: &[Pubkey]| {
let mut purges_in_root = Vec::new();
let mut purges = HashMap::new();
let mut purges_zero_lamports = HashMap::new();
let mut purges_old_accounts = Vec::new();
for pubkey in pubkeys {
match self.accounts_index.get(pubkey, None, max_clean_root) {
AccountIndexGetResult::Found(locked_entry, index) => {
let slot_list = locked_entry.slot_list();
let (slot, account_info) = &slot_list[index];
if account_info.lamports == 0 {
purges.insert(
purges_zero_lamports.insert(
*pubkey,
self.accounts_index
.roots_and_ref_count(&locked_entry, max_clean_root),
@ -1624,11 +1623,17 @@ impl AccountsDb {
if let Some(max_clean_root) = max_clean_root {
assert!(slot <= max_clean_root);
}
purges_in_root.push(*pubkey);
purges_old_accounts.push(*pubkey);
}
}
AccountIndexGetResult::NotFoundOnFork => {
// do nothing - pubkey is in index, but not found in a root slot
// This pubkey is in the index but not in a root slot, so clean
// it up by adding it to the to-be-purged list.
//
// Also, this pubkey must have been touched by some slot since
// it was in the dirty list, so we assume that the slot it was
// touched in must be unrooted.
purges_old_accounts.push(*pubkey);
}
AccountIndexGetResult::Missing(lock) => {
// pubkey is missing from index, so remove from zero_lamports_list
@ -1637,7 +1642,7 @@ impl AccountsDb {
}
};
}
(purges, purges_in_root)
(purges_zero_lamports, purges_old_accounts)
})
.reduce(
|| (HashMap::new(), Vec::new()),
@ -1654,7 +1659,7 @@ impl AccountsDb {
let mut clean_old_rooted = Measure::start("clean_old_roots");
let (purged_account_slots, removed_accounts) =
self.clean_old_rooted_accounts(purges_in_root, max_clean_root);
self.clean_accounts_older_than_root(purges_old_accounts, max_clean_root);
if self.caching_enabled {
self.do_reset_uncleaned_roots(max_clean_root);
@ -1668,7 +1673,7 @@ impl AccountsDb {
// Calculate store counts as if everything was purged
// Then purge if we can
let mut store_counts: HashMap<AppendVecId, (usize, HashSet<Pubkey>)> = HashMap::new();
for (key, (account_infos, ref_count)) in purges.iter_mut() {
for (key, (account_infos, ref_count)) in purges_zero_lamports.iter_mut() {
if purged_account_slots.contains_key(&key) {
*ref_count = self.accounts_index.ref_count_from_storage(&key);
}
@ -1683,7 +1688,7 @@ impl AccountsDb {
return false;
}
// Check if this update in `slot` to the account with `key` was reclaimed earlier by
// `clean_old_rooted_accounts()`
// `clean_accounts_older_than_root()`
let was_reclaimed = removed_accounts
.get(&account_info.store_id)
.map(|store_removed| store_removed.contains(&account_info.offset))
@ -1714,13 +1719,13 @@ impl AccountsDb {
store_counts_time.stop();
let mut calc_deps_time = Measure::start("calc_deps");
Self::calc_delete_dependencies(&purges, &mut store_counts);
Self::calc_delete_dependencies(&purges_zero_lamports, &mut store_counts);
calc_deps_time.stop();
// Only keep purges where the entire history of the account in the root set
// Only keep purges_zero_lamports where the entire history of the account in the root set
// can be purged. All AppendVecs for those updates are dead.
let mut purge_filter = Measure::start("purge_filter");
purges.retain(|_pubkey, (account_infos, _ref_count)| {
purges_zero_lamports.retain(|_pubkey, (account_infos, _ref_count)| {
for (_slot, account_info) in account_infos.iter() {
if store_counts.get(&account_info.store_id).unwrap().0 != 0 {
return false;
@ -1732,7 +1737,7 @@ impl AccountsDb {
let mut reclaims_time = Measure::start("reclaims");
// Recalculate reclaims with new purge set
let pubkey_to_slot_set: Vec<_> = purges
let pubkey_to_slot_set: Vec<_> = purges_zero_lamports
.into_iter()
.map(|(key, (slots_list, _ref_count))| {
(
@ -2513,7 +2518,7 @@ impl AccountsDb {
// purge_slot_cache_pubkeys() | (removes existing store_id, offset for caches)
// OR |
// clean_accounts()/ |
// clean_old_rooted_accounts() | (removes existing store_id, offset for stores)
// clean_accounts_older_than_root()| (removes existing store_id, offset for stores)
// V
//
// Remarks for purger: So, for any reading operations, it's a race condition
@ -3076,9 +3081,6 @@ impl AccountsDb {
// It should not be possible that a slot is neither in the cache or storage. Even in
// a slot with all ticks, `Bank::new_from_parent()` immediately stores some sysvars
// on bank creation.
// Remove any delta pubkey set if existing.
self.uncleaned_pubkeys.remove(remove_slot);
}
remove_storages_elapsed.stop();

View File

@ -1025,7 +1025,7 @@ impl<T: 'static + Clone + IsCached + ZeroLamport> AccountsIndex<T> {
}
// Get the maximum root <= `max_allowed_root` from the given `slice`
fn get_max_root(
fn get_newest_root_in_slot_list(
roots: &RollingBitField,
slice: SlotSlice<T>,
max_allowed_root: Option<Slot>,
@ -1202,24 +1202,26 @@ impl<T: 'static + Clone + IsCached + ZeroLamport> AccountsIndex<T> {
fn purge_older_root_entries(
&self,
pubkey: &Pubkey,
list: &mut SlotList<T>,
slot_list: &mut SlotList<T>,
reclaims: &mut SlotList<T>,
max_clean_root: Option<Slot>,
account_indexes: &HashSet<AccountIndex>,
) {
let roots_tracker = &self.roots_tracker.read().unwrap();
let max_root = Self::get_max_root(&roots_tracker.roots, &list, max_clean_root);
let newest_root_in_slot_list =
Self::get_newest_root_in_slot_list(&roots_tracker.roots, &slot_list, max_clean_root);
let max_clean_root = max_clean_root.unwrap_or(roots_tracker.max_root);
let mut purged_slots: HashSet<Slot> = HashSet::new();
list.retain(|(slot, value)| {
let should_purge = Self::can_purge(max_root, *slot) && !value.is_cached();
slot_list.retain(|(slot, value)| {
let should_purge =
Self::can_purge_older_entries(max_clean_root, newest_root_in_slot_list, *slot)
&& !value.is_cached();
if should_purge {
reclaims.push((*slot, value.clone()));
purged_slots.insert(*slot);
false
} else {
true
}
!should_purge
});
self.purge_secondary_indexes_by_inner_key(pubkey, Some(&purged_slots), account_indexes);
@ -1232,6 +1234,7 @@ impl<T: 'static + Clone + IsCached + ZeroLamport> AccountsIndex<T> {
max_clean_root: Option<Slot>,
account_indexes: &HashSet<AccountIndex>,
) {
let mut is_slot_list_empty = false;
if let Some(mut locked_entry) = self.get_account_write_entry(pubkey) {
locked_entry.slot_list_mut(|slot_list| {
self.purge_older_root_entries(
@ -1241,12 +1244,42 @@ impl<T: 'static + Clone + IsCached + ZeroLamport> AccountsIndex<T> {
max_clean_root,
account_indexes,
);
is_slot_list_empty = slot_list.is_empty();
});
}
// If the slot list is empty, remove the pubkey from `account_maps`. Make sure to grab the
// lock and double check the slot list is still empty, because another writer could have
// locked and inserted the pubkey inbetween when `is_slot_list_empty=true` and the call to
// remove() below.
if is_slot_list_empty {
let mut w_maps = self.account_maps.write().unwrap();
if let Some(x) = w_maps.get(pubkey) {
if x.slot_list.read().unwrap().is_empty() {
w_maps.remove(pubkey);
}
}
}
}
pub fn can_purge(max_root: Slot, slot: Slot) -> bool {
slot < max_root
/// When can an entry be purged?
///
/// If we get a slot update where slot != newest_root_in_slot_list for an account where slot <
/// max_clean_root, then we know it's safe to delete because:
///
/// a) If slot < newest_root_in_slot_list, then we know the update is outdated by a later rooted
/// update, namely the one in newest_root_in_slot_list
///
/// b) If slot > newest_root_in_slot_list, then because slot < max_clean_root and we know there are
/// no roots in the slot list between newest_root_in_slot_list and max_clean_root, (otherwise there
/// would be a bigger newest_root_in_slot_list, which is a contradiction), then we know slot must be
/// an unrooted slot less than max_clean_root and thus safe to clean as well.
fn can_purge_older_entries(
max_clean_root: Slot,
newest_root_in_slot_list: Slot,
slot: Slot,
) -> bool {
slot < max_clean_root && slot != newest_root_in_slot_list
}
pub fn is_root(&self, slot: Slot) -> bool {

View File

@ -12470,4 +12470,54 @@ pub(crate) mod tests {
))
);
}
#[test]
fn test_clean_unrooted_dropped_banks() {
//! Test that unrooted banks are cleaned up properly
//!
//! slot 0: bank0 (rooted)
//! / \
//! slot 1: / bank1 (unrooted and dropped)
//! /
//! slot 2: bank2 (rooted)
//!
//! In the scenario above, when `clean_accounts()` is called on bank2, the keys that exist
//! _only_ in bank1 should be cleaned up, since those keys are unreachable.
//
solana_logger::setup();
let (genesis_config, mint_keypair) = create_genesis_config(100);
let bank0 = Arc::new(Bank::new(&genesis_config));
let collector = Pubkey::new_unique();
let pubkey1 = Pubkey::new_unique();
let pubkey2 = Pubkey::new_unique();
bank0.transfer(2, &mint_keypair, &pubkey2).unwrap();
bank0.freeze();
let slot = 1;
let bank1 = Bank::new_from_parent(&bank0, &collector, slot);
bank1.transfer(3, &mint_keypair, &pubkey1).unwrap();
bank1.freeze();
let slot = slot + 1;
let bank2 = Bank::new_from_parent(&bank0, &collector, slot);
bank2.transfer(4, &mint_keypair, &pubkey2).unwrap();
bank2.freeze(); // the freeze here is not strictly necessary, but more for illustration
bank2.squash();
drop(bank1);
bank2.clean_accounts(false);
assert_eq!(
bank2
.rc
.accounts
.accounts_db
.accounts_index
.ref_count_from_storage(&pubkey1),
0
);
}
}