diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 6520d4de189d64..62de2fbd61660c 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -475,6 +475,7 @@ pub(crate) struct ShrinkCollect<'a, T: ShrinkCollectRefs<'a>> { pub(crate) slot: Slot, pub(crate) capacity: u64, pub(crate) unrefed_pubkeys: Vec<&'a Pubkey>, + pub(crate) zero_lamport_single_ref_pubkeys: Vec<&'a Pubkey>, pub(crate) alive_accounts: T, /// total size in storage of all alive accounts pub(crate) alive_total_bytes: usize, @@ -524,6 +525,8 @@ struct LoadAccountsIndexForShrink<'a, T: ShrinkCollectRefs<'a>> { alive_accounts: T, /// pubkeys that were unref'd in the accounts index because they were dead unrefed_pubkeys: Vec<&'a Pubkey>, + /// pubkeys that are the last remaining zero lamport instance of an account + zero_lamport_single_ref_pubkeys: Vec<&'a Pubkey>, /// true if all alive accounts are zero lamport accounts all_are_zero_lamports: bool, /// index entries we need to hold onto to keep them from getting flushed @@ -2011,6 +2014,7 @@ pub struct ShrinkStats { dead_accounts: AtomicU64, alive_accounts: AtomicU64, accounts_loaded: AtomicU64, + purged_zero_lamports: AtomicU64, } impl ShrinkStats { @@ -2109,6 +2113,11 @@ impl ShrinkStats { self.accounts_loaded.swap(0, Ordering::Relaxed) as i64, i64 ), + ( + "purged_zero_lamports_count", + self.purged_zero_lamports.swap(0, Ordering::Relaxed), + i64 + ), ); } } @@ -2309,6 +2318,13 @@ impl ShrinkAncientStats { self.many_refs_old_alive.swap(0, Ordering::Relaxed), i64 ), + ( + "purged_zero_lamports_count", + self.shrink_stats + .purged_zero_lamports + .swap(0, Ordering::Relaxed), + i64 + ), ); } } @@ -3793,12 +3809,14 @@ impl AccountsDb { let count = accounts.len(); let mut alive_accounts = T::with_capacity(count, slot_to_shrink); let mut unrefed_pubkeys = Vec::with_capacity(count); + let mut zero_lamport_single_ref_pubkeys = Vec::with_capacity(count); let mut alive = 0; let mut dead = 0; let mut index = 0; let mut all_are_zero_lamports = true; let mut index_entries_being_shrunk = Vec::with_capacity(accounts.len()); + let latest_full_snapshot_slot = self.latest_full_snapshot_slot(); self.accounts_index.scan( accounts.iter().map(|account| account.pubkey()), |pubkey, slots_refs, entry| { @@ -3817,6 +3835,21 @@ impl AccountsDb { unrefed_pubkeys.push(pubkey); result = AccountsIndexScanResult::Unref; dead += 1; + } else if stored_account.is_zero_lamport() + && ref_count == 1 + && latest_full_snapshot_slot + .map(|latest_full_snapshot_slot| { + latest_full_snapshot_slot >= slot_to_shrink + }) + .unwrap_or(true) + { + // only do this if our slot is prior to the latest full snapshot + // we found a zero lamport account that is the only instance of this account. We can delete it completely. + zero_lamport_single_ref_pubkeys.push(pubkey); + self.add_uncleaned_pubkeys_after_shrink( + slot_to_shrink, + [*pubkey].into_iter(), + ); } else { // Hold onto the index entry arc so that it cannot be flushed. // Since we are shrinking these entries, we need to disambiguate storage ids during this period and those only exist in the in-memory accounts index. @@ -3839,6 +3872,7 @@ impl AccountsDb { LoadAccountsIndexForShrink { alive_accounts, unrefed_pubkeys, + zero_lamport_single_ref_pubkeys, all_are_zero_lamports, index_entries_being_shrunk, } @@ -3939,6 +3973,7 @@ impl AccountsDb { let len = stored_accounts.len(); let alive_accounts_collect = Mutex::new(T::with_capacity(len, slot)); let unrefed_pubkeys_collect = Mutex::new(Vec::with_capacity(len)); + let zero_lamport_single_ref_pubkeys_collect = Mutex::new(Vec::with_capacity(len)); stats .accounts_loaded .fetch_add(len as u64, Ordering::Relaxed); @@ -3955,6 +3990,7 @@ impl AccountsDb { alive_accounts, mut unrefed_pubkeys, all_are_zero_lamports, + mut zero_lamport_single_ref_pubkeys, mut index_entries_being_shrunk, } = self.load_accounts_index_for_shrink(stored_accounts, stats, slot); @@ -3967,6 +4003,10 @@ impl AccountsDb { .lock() .unwrap() .append(&mut unrefed_pubkeys); + zero_lamport_single_ref_pubkeys_collect + .lock() + .unwrap() + .append(&mut zero_lamport_single_ref_pubkeys); index_entries_being_shrunk_outer .lock() .unwrap() @@ -3979,6 +4019,9 @@ impl AccountsDb { let alive_accounts = alive_accounts_collect.into_inner().unwrap(); let unrefed_pubkeys = unrefed_pubkeys_collect.into_inner().unwrap(); + let zero_lamport_single_ref_pubkeys = zero_lamport_single_ref_pubkeys_collect + .into_inner() + .unwrap(); index_read_elapsed.stop(); stats @@ -4002,6 +4045,7 @@ impl AccountsDb { slot, capacity: *capacity, unrefed_pubkeys, + zero_lamport_single_ref_pubkeys, alive_accounts, alive_total_bytes, total_starting_accounts: len, @@ -4010,6 +4054,41 @@ impl AccountsDb { } } + /// These accounts were found during shrink of `slot` to be slot_list=[slot] and ref_count == 1 and lamports = 0. + /// This means this slot contained the only account data for this pubkey and it is zero lamport. + /// Thus, we did NOT treat this as an alive account, so we did NOT copy the zero lamport account to the new + /// storage. So, the account will no longer be alive or exist at `slot`. + /// So, first, remove the ref count since this newly shrunk storage will no longer access it. + /// Second, remove `slot` from the index entry's slot list. If the slot list is now empty, then the + /// pubkey can be removed completely from the index. + /// In parallel with this code (which is running in the bg), the same pubkey could be revived and written to + /// as part of tx processing. In that case, the slot list will contain a slot in the write cache and the + /// index entry will NOT be deleted. + fn remove_zero_lamport_single_ref_accounts_after_shrink( + &self, + zero_lamport_single_ref_pubkeys: &[&Pubkey], + slot: Slot, + stats: &ShrinkStats, + ) { + stats.purged_zero_lamports.fetch_add( + zero_lamport_single_ref_pubkeys.len() as u64, + Ordering::Relaxed, + ); + + // we have to unref before we `purge_keys_exact`. Otherwise, we could race with the foreground with tx processing + // reviving this index entry and then we'd unref the revived version, which is a refcount bug. + self.accounts_index.scan( + zero_lamport_single_ref_pubkeys.iter().cloned(), + |_pubkey, _slots_refs, _entry| AccountsIndexScanResult::Unref, + Some(AccountsIndexScanResult::Unref), + false, + ); + + zero_lamport_single_ref_pubkeys.iter().for_each(|k| { + _ = self.purge_keys_exact([&(**k, slot)].into_iter()); + }); + } + /// common code from shrink and combine_ancient_slots /// get rid of all original store_ids in the slot pub(crate) fn remove_old_stores_shrink<'a, T: ShrinkCollectRefs<'a>>( @@ -4020,7 +4099,19 @@ impl AccountsDb { shrink_can_be_active: bool, ) { let mut time = Measure::start("remove_old_stores_shrink"); + + // handle the zero lamport alive accounts before calling clean + // We have to update the index entries for these zero lamport pubkeys before we remove the storage in `mark_dirty_dead_stores` + // that contained the accounts. + self.remove_zero_lamport_single_ref_accounts_after_shrink( + &shrink_collect.zero_lamport_single_ref_pubkeys, + shrink_collect.slot, + stats, + ); + // Purge old, overwritten storage entries + // This has the side effect of dropping `shrink_in_progress`, which removes the old storage completely. The + // index has to be correct before we drop the old storage. let dead_storages = self.mark_dirty_dead_stores( shrink_collect.slot, // If all accounts are zero lamports, then we want to mark the entire OLD append vec as dirty. @@ -10884,6 +10975,146 @@ pub mod tests { assert_eq!(accounts.alive_account_count_in_slot(1), 0); } + #[test] + fn test_remove_zero_lamport_single_ref_accounts_after_shrink() { + for pass in 0..3 { + let accounts = AccountsDb::new_single_for_tests(); + let pubkey_zero = Pubkey::from([1; 32]); + let pubkey2 = Pubkey::from([2; 32]); + let account = AccountSharedData::new(1, 0, AccountSharedData::default().owner()); + let zero_lamport_account = + AccountSharedData::new(0, 0, AccountSharedData::default().owner()); + let slot = 1; + + accounts.store_for_tests( + slot, + &[(&pubkey_zero, &zero_lamport_account), (&pubkey2, &account)], + ); + + // Simulate rooting the zero-lamport account, writes it to storage + accounts.calculate_accounts_delta_hash(slot); + accounts.add_root_and_flush_write_cache(slot); + + if pass > 0 { + // store in write cache + accounts.store_for_tests(slot + 1, &[(&pubkey_zero, &zero_lamport_account)]); + if pass == 2 { + // move to a storage (causing ref count to increase) + accounts.calculate_accounts_delta_hash(slot + 1); + accounts.add_root_and_flush_write_cache(slot + 1); + } + } + + accounts.accounts_index.get_and_then(&pubkey_zero, |entry| { + let expected_ref_count = if pass < 2 { 1 } else { 2 }; + assert_eq!(entry.unwrap().ref_count(), expected_ref_count, "{pass}"); + let expected_slot_list = if pass < 1 { 1 } else { 2 }; + assert_eq!( + entry.unwrap().slot_list.read().unwrap().len(), + expected_slot_list + ); + (false, ()) + }); + accounts.accounts_index.get_and_then(&pubkey2, |entry| { + assert!(entry.is_some()); + (false, ()) + }); + + let zero_lamport_single_ref_pubkeys = [&pubkey_zero]; + accounts.remove_zero_lamport_single_ref_accounts_after_shrink( + &zero_lamport_single_ref_pubkeys, + slot, + &ShrinkStats::default(), + ); + + accounts.accounts_index.get_and_then(&pubkey_zero, |entry| { + if pass == 0 { + // should not exist in index at all + assert!(entry.is_none(), "{pass}"); + } else { + // alive only in slot + 1 + assert_eq!(entry.unwrap().slot_list.read().unwrap().len(), 1); + assert_eq!( + entry + .unwrap() + .slot_list + .read() + .unwrap() + .first() + .map(|(s, _)| s) + .cloned() + .unwrap(), + slot + 1 + ); + // refcount = 1 if we flushed the write cache for slot + 1 + let expected_ref_count = if pass < 2 { 0 } else { 1 }; + assert_eq!( + entry.map(|e| e.ref_count()), + Some(expected_ref_count), + "{pass}" + ); + } + (false, ()) + }); + + accounts.accounts_index.get_and_then(&pubkey2, |entry| { + assert!(entry.is_some(), "{pass}"); + (false, ()) + }); + } + } + + #[test] + fn test_shrink_zero_lamport_single_ref_account() { + solana_logger::setup(); + + // store a zero and non-zero lamport account + // make sure clean marks the ref_count=1, zero lamport account dead and removes pubkey from index completely + let accounts = AccountsDb::new_single_for_tests(); + let pubkey_zero = Pubkey::from([1; 32]); + let pubkey2 = Pubkey::from([2; 32]); + let account = AccountSharedData::new(1, 0, AccountSharedData::default().owner()); + let zero_lamport_account = + AccountSharedData::new(0, 0, AccountSharedData::default().owner()); + + // Store a zero-lamport account and a non-zero lamport account + accounts.store_for_tests( + 1, + &[(&pubkey_zero, &zero_lamport_account), (&pubkey2, &account)], + ); + + // Simulate rooting the zero-lamport account, should be a + // candidate for cleaning + accounts.calculate_accounts_delta_hash(1); + accounts.add_root_and_flush_write_cache(1); + + // for testing, we need to cause shrink to think this will be productive. + // The zero lamport account isn't dead, but it can become dead inside shrink. + accounts + .storage + .get_slot_storage_entry(1) + .unwrap() + .alive_bytes + .fetch_sub(aligned_stored_size(0), Ordering::Relaxed); + + // Slot 1 should be cleaned, but + // zero-lamport account should not be cleaned since last full snapshot root is before slot 1 + accounts.shrink_slot_forced(1); + + assert!(accounts.storage.get_slot_storage_entry(1).is_some()); + + // the zero lamport account should be marked as dead + assert_eq!(accounts.alive_account_count_in_slot(1), 1); + + // zero lamport account should be dead in the index + assert!(!accounts + .accounts_index + .contains_with(&pubkey_zero, None, None)); + // other account should still be alive + assert!(accounts.accounts_index.contains_with(&pubkey2, None, None)); + assert!(accounts.storage.get_slot_storage_entry(1).is_some()); + } + #[test] fn test_clean_multiple_zero_lamport_decrements_index_ref_count() { solana_logger::setup(); @@ -15910,6 +16141,8 @@ pub mod tests { debug!("space: {space}, lamports: {lamports}, alive: {alive}, account_count: {account_count}, append_opposite_alive_account: {append_opposite_alive_account}, append_opposite_zero_lamport_account: {append_opposite_zero_lamport_account}, normal_account_count: {normal_account_count}"); let db = AccountsDb::new_single_for_tests(); let slot5 = 5; + // don't do special zero lamport account handling + db.set_latest_full_snapshot_slot(0); let mut account = AccountSharedData::new( lamports, space, diff --git a/accounts-db/src/ancient_append_vecs.rs b/accounts-db/src/ancient_append_vecs.rs index 998781d8c04850..ffe025e7474a39 100644 --- a/accounts-db/src/ancient_append_vecs.rs +++ b/accounts-db/src/ancient_append_vecs.rs @@ -3836,6 +3836,7 @@ pub mod tests { unrefed_pubkeys: unrefed_pubkeys.iter().collect(), // irrelevant fields + zero_lamport_single_ref_pubkeys: Vec::default(), slot: 0, capacity: 0, alive_accounts: ShrinkCollectAliveSeparatedByRefs {