From a601a9db0341b92b6ceffe7ce2058cc13b51968c Mon Sep 17 00:00:00 2001 From: jeff washington Date: Mon, 9 Sep 2024 15:30:49 -0500 Subject: [PATCH 1/7] check for all zeros on startup --- accounts-db/src/accounts_db.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 02449fcc021c33..f3c1c3a86543df 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -8833,7 +8833,7 @@ impl AccountsDb { fn generate_index_for_slot( &self, - storage: &AccountStorageEntry, + storage: &Arc, slot: Slot, store_id: AccountsFileId, rent_collector: &RentCollector, @@ -8850,15 +8850,22 @@ impl AccountsDb { let mut amount_to_top_off_rent = 0; let mut stored_size_alive = 0; + let mut all_accounts_are_zero_lamports = true; + let (dirty_pubkeys, insert_time_us, mut generate_index_results) = { let mut items_local = Vec::default(); storage.accounts.scan_index(|info| { stored_size_alive += info.stored_size_aligned; if info.index_info.lamports > 0 { accounts_data_len += info.index_info.data_len; + all_accounts_are_zero_lamports = false; } items_local.push(info.index_info); }); + if all_accounts_are_zero_lamports { + self.dirty_stores.insert(slot, Arc::clone(storage)); + log::error!("all zeros: {slot}"); + } let items = items_local.into_iter().map(|info| { if let Some(amount_to_top_off_rent_this_account) = Self::stats_for_rent_payers( &info.pubkey, From 16260a3cd1cbe99eb4882745f9d5189812962a07 Mon Sep 17 00:00:00 2001 From: jeff washington Date: Tue, 10 Sep 2024 10:13:22 -0500 Subject: [PATCH 2/7] uncleaned roots --- accounts-db/src/accounts_db.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index f3c1c3a86543df..7f9caa3bbeb611 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -8863,8 +8863,9 @@ impl AccountsDb { items_local.push(info.index_info); }); if all_accounts_are_zero_lamports { + // this whole slot can likely be marked dead and dropped. Clean has to determine that. There could be an older non-zero account for any of these zero lamport accounts. self.dirty_stores.insert(slot, Arc::clone(storage)); - log::error!("all zeros: {slot}"); + self.accounts_index.add_uncleaned_roots([slot].into_iter()); } let items = items_local.into_iter().map(|info| { if let Some(amount_to_top_off_rent_this_account) = Self::stats_for_rent_payers( From 1a9f861e06a2081bb5b60ecb55650845c7af3a4d Mon Sep 17 00:00:00 2001 From: jeff washington Date: Wed, 28 Aug 2024 17:35:44 -0500 Subject: [PATCH 3/7] log oldest non ancient slot --- accounts-db/src/accounts_db.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 7f9caa3bbeb611..2bb257b2667c5b 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -3053,7 +3053,9 @@ impl AccountsDb { result, -((epoch_schedule.slots_per_epoch as i64).saturating_sub(1)), ); - result.min(max_root_inclusive) + let result = result.min(max_root_inclusive); + log::error!("oldest non ancient slot: {result}, max root: {max_root_inclusive}, diff: {}, offset: {:?}", max_root_inclusive.saturating_sub(result), self.ancient_append_vec_offset); + result } /// Collect all the uncleaned slots, up to a max slot From 352610155790977567a2fc8b876bfdb8d3f1482c Mon Sep 17 00:00:00 2001 From: jeff washington Date: Wed, 28 Aug 2024 17:45:25 -0500 Subject: [PATCH 4/7] log slot of clean and ancient pack --- accounts-db/src/accounts_db.rs | 7 +++++++ accounts-db/src/ancient_append_vecs.rs | 1 + 2 files changed, 8 insertions(+) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 2bb257b2667c5b..a3bfd161f7ffe3 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -1991,6 +1991,7 @@ pub(crate) struct ShrinkAncientStats { pub(crate) many_refs_old_alive: AtomicU64, pub(crate) slots_eligible_to_shrink: AtomicU64, pub(crate) total_dead_bytes: AtomicU64, + pub(crate) slot: AtomicU64, pub(crate) total_alive_bytes: AtomicU64, } @@ -2323,6 +2324,11 @@ impl ShrinkAncientStats { self.slots_eligible_to_shrink.swap(0, Ordering::Relaxed), i64 ), + ( + "slot", + self.slot.swap(0, Ordering::Relaxed), + i64 + ), ( "total_dead_bytes", self.total_dead_bytes.swap(0, Ordering::Relaxed), @@ -3599,6 +3605,7 @@ impl AccountsDb { self.clean_accounts_stats.report(); datapoint_info!( "clean_accounts", + ("slot", max_clean_root_inclusive.unwrap_or_default(), i64), ("total_us", measure_all.as_us(), i64), ( "collect_delta_keys_us", diff --git a/accounts-db/src/ancient_append_vecs.rs b/accounts-db/src/ancient_append_vecs.rs index 0eb3309ac5133a..b69eb161e95cd4 100644 --- a/accounts-db/src/ancient_append_vecs.rs +++ b/accounts-db/src/ancient_append_vecs.rs @@ -393,6 +393,7 @@ impl AccountsDb { tuning: PackedAncientStorageTuning, metrics: &mut ShrinkStatsSub, ) { + self.shrink_ancient_stats.slot.store(sorted_slots.first().cloned().unwrap_or_default(), Ordering::Relaxed); self.shrink_ancient_stats .slots_considered .fetch_add(sorted_slots.len() as u64, Ordering::Relaxed); From a84f6fefe15951c8173b800d8a2d9d6e8737091d Mon Sep 17 00:00:00 2001 From: jeff washington Date: Tue, 3 Sep 2024 15:07:46 -0500 Subject: [PATCH 5/7] add uncleaned roots when we shrink away non-zero lamport accounts --- accounts-db/src/accounts_db.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index a3bfd161f7ffe3..82db2592ba81d2 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -2042,6 +2042,7 @@ pub struct ShrinkStats { purged_zero_lamports: AtomicU64, accounts_not_found_in_index: AtomicU64, num_ancient_slots_shrunk: AtomicU64, + adding_slots_to_clean: AtomicU64, } impl ShrinkStats { @@ -2165,6 +2166,11 @@ impl ShrinkStats { self.accounts_not_found_in_index.swap(0, Ordering::Relaxed), i64 ), + ( + "adding_slots_to_clean", + self.adding_slots_to_clean.swap(0, Ordering::Relaxed), + i64 + ), ); } } @@ -2403,6 +2409,13 @@ impl ShrinkAncientStats { .swap(0, Ordering::Relaxed), i64 ), + ( + "adding_slots_to_clean", + self.shrink_stats + .adding_slots_to_clean + .swap(0, Ordering::Relaxed), + i64 + ), ); } } @@ -3981,6 +3994,7 @@ impl AccountsDb { let mut alive = 0; let mut dead = 0; let mut index = 0; + let mut adding_slots_to_clean = 0; let mut index_scan_returned_some_count = 0; let mut index_scan_returned_none_count = 0; let mut all_are_zero_lamports = true; @@ -4025,6 +4039,18 @@ impl AccountsDb { // rewriting the storage entries. pubkeys_to_unref.push(pubkey); dead += 1; + + // If we are marking something dead, and the only remaining alive account is zero lamport, then make that zero lamport slot ready to be cleaned. + // If that slot happens to only contain zero lamport accounts, the whole slot will go away + if slot_list.len() == 1 // should we also check for ref counts here? + && slot_list + .iter() + .all(|(_slot, acct_info)| acct_info.is_zero_lamport()) + { + adding_slots_to_clean += 1; + self.accounts_index + .add_uncleaned_roots(slot_list.iter().map(|(slot, _)| *slot)); + } } else { do_populate_accounts_for_shrink(ref_count, slot_list); } @@ -4055,6 +4081,9 @@ impl AccountsDb { .fetch_add(index_scan_returned_none_count, Ordering::Relaxed); stats.alive_accounts.fetch_add(alive, Ordering::Relaxed); stats.dead_accounts.fetch_add(dead, Ordering::Relaxed); + stats + .adding_slots_to_clean + .fetch_add(adding_slots_to_clean, Ordering::Relaxed); LoadAccountsIndexForShrink { alive_accounts, From 85413834cebb9bc5ad55a9ca516ec7be923ec660 Mon Sep 17 00:00:00 2001 From: jeff washington Date: Fri, 6 Sep 2024 17:40:38 -0500 Subject: [PATCH 6/7] adding dead slots to clean and such --- accounts-db/src/accounts_db.rs | 106 ++++++++++++++++++++++-------- accounts-db/src/accounts_file.rs | 7 ++ accounts-db/src/append_vec.rs | 4 ++ accounts-db/src/tiered_storage.rs | 5 ++ 4 files changed, 96 insertions(+), 26 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 82db2592ba81d2..00c4f6394f1962 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -1091,6 +1091,10 @@ pub struct AccountStorageEntry { approx_store_count: AtomicUsize, alive_bytes: AtomicUsize, + + /// offsets to accounts that are zero lamport single ref stored in this storage. + /// These are still alive. But, shrink will be able to remove them. + zero_lamport_single_ref_offsets: RwLock>, } impl AccountStorageEntry { @@ -1112,6 +1116,7 @@ impl AccountStorageEntry { count_and_status: SeqLock::new((0, AccountStorageStatus::Available)), approx_store_count: AtomicUsize::new(0), alive_bytes: AtomicUsize::new(0), + zero_lamport_single_ref_offsets: RwLock::default(), } } @@ -1130,6 +1135,7 @@ impl AccountStorageEntry { approx_store_count: AtomicUsize::new(self.approx_stored_count()), alive_bytes: AtomicUsize::new(self.alive_bytes()), accounts, + zero_lamport_single_ref_offsets: RwLock::default(), }) } @@ -1146,6 +1152,7 @@ impl AccountStorageEntry { count_and_status: SeqLock::new((0, AccountStorageStatus::Available)), approx_store_count: AtomicUsize::new(num_accounts), alive_bytes: AtomicUsize::new(0), + zero_lamport_single_ref_offsets: RwLock::default(), } } @@ -2042,7 +2049,9 @@ pub struct ShrinkStats { purged_zero_lamports: AtomicU64, accounts_not_found_in_index: AtomicU64, num_ancient_slots_shrunk: AtomicU64, - adding_slots_to_clean: AtomicU64, + adding_dead_slots_to_clean: AtomicU64, + adding_shrink_slots_from_zeros: AtomicU64, + marking_zero_dead_accounts: AtomicU64, } impl ShrinkStats { @@ -2167,8 +2176,18 @@ impl ShrinkStats { i64 ), ( - "adding_slots_to_clean", - self.adding_slots_to_clean.swap(0, Ordering::Relaxed), + "adding_dead_slots_to_clean", + self.adding_dead_slots_to_clean.swap(0, Ordering::Relaxed), + i64 + ), + ( + "adding_shrink_slots_from_zeros", + self.adding_shrink_slots_from_zeros.swap(0, Ordering::Relaxed), + i64 + ), + ( + "marking_zero_dead_accounts", + self.marking_zero_dead_accounts.swap(0, Ordering::Relaxed), i64 ), ); @@ -2409,13 +2428,6 @@ impl ShrinkAncientStats { .swap(0, Ordering::Relaxed), i64 ), - ( - "adding_slots_to_clean", - self.shrink_stats - .adding_slots_to_clean - .swap(0, Ordering::Relaxed), - i64 - ), ); } } @@ -4042,14 +4054,16 @@ impl AccountsDb { // If we are marking something dead, and the only remaining alive account is zero lamport, then make that zero lamport slot ready to be cleaned. // If that slot happens to only contain zero lamport accounts, the whole slot will go away - if slot_list.len() == 1 // should we also check for ref counts here? - && slot_list - .iter() - .all(|(_slot, acct_info)| acct_info.is_zero_lamport()) - { - adding_slots_to_clean += 1; - self.accounts_index - .add_uncleaned_roots(slot_list.iter().map(|(slot, _)| *slot)); + if slot_list.len() == 1 { + // should we also check for ref counts here? + if let Some((slot_alive, acct_info)) = slot_list.first() { + if acct_info.is_zero_lamport() && !acct_info.is_cached() { + self.zero_lamport_single_ref_found( + *slot_alive, + acct_info.offset(), + ); + } + } } } else { do_populate_accounts_for_shrink(ref_count, slot_list); @@ -4081,9 +4095,6 @@ impl AccountsDb { .fetch_add(index_scan_returned_none_count, Ordering::Relaxed); stats.alive_accounts.fetch_add(alive, Ordering::Relaxed); stats.dead_accounts.fetch_add(dead, Ordering::Relaxed); - stats - .adding_slots_to_clean - .fetch_add(adding_slots_to_clean, Ordering::Relaxed); LoadAccountsIndexForShrink { alive_accounts, @@ -4391,6 +4402,37 @@ impl AccountsDb { ); } + pub(crate) fn zero_lamport_single_ref_found(&self, slot: Slot, offset: usize) { + if let Some(store) = self.storage.get_slot_storage_entry(slot) { + let mut zero_lamport_single_ref_offsets = + store.zero_lamport_single_ref_offsets.write().unwrap(); + if zero_lamport_single_ref_offsets.insert(offset) { + // this wasn't previously marked as zero lamport single ref + if zero_lamport_single_ref_offsets.len() == store.count() { + // all accounts in this storage can be dead + self.accounts_index.add_uncleaned_roots([slot].into_iter()); + self.shrink_stats + .adding_dead_slots_to_clean + .fetch_add(1, Ordering::Relaxed); + } else { + if Self::is_shrinking_productive(slot, &store) + && self.is_candidate_for_shrink(&store) + { + // this store might be eligible for shrinking now + self.shrink_candidate_slots.lock().unwrap().insert(slot); + self.shrink_stats + .adding_shrink_slots_from_zeros + .fetch_add(1, Ordering::Relaxed); + } else { + self.shrink_stats + .marking_zero_dead_accounts + .fetch_add(1, Ordering::Relaxed); + } + } + } + } + } + fn do_shrink_slot_store(&self, slot: Slot, store: &Arc) { if self.accounts_cache.contains(slot) { // It is not correct to shrink a slot while it is in the write cache until flush is complete and the slot is removed from the write cache. @@ -8072,7 +8114,14 @@ impl AccountsDb { let alive_bytes = store.alive_bytes() as u64; let total_bytes = store.capacity(); - if Self::should_not_shrink(alive_bytes, total_bytes) { + let zero_lamport_dead_bytes = store.accounts.dead_bytes_due_to_zero_lamport_single_ref( + store.zero_lamport_single_ref_offsets.read().unwrap().len(), + ); + + if Self::should_not_shrink( + alive_bytes.saturating_sub(zero_lamport_dead_bytes as u64), + total_bytes, + ) { trace!( "shrink_slot_forced ({}): not able to shrink at all: alive/stored: {}/{} ({}b / {}b) save: {}", slot, @@ -8099,12 +8148,17 @@ impl AccountsDb { } else { store.capacity() }; + + let zero_lamport_dead_bytes = store.accounts.dead_bytes_due_to_zero_lamport_single_ref( + store.zero_lamport_single_ref_offsets.read().unwrap().len(), + ); + + let alive_bytes = store.alive_bytes().saturating_sub(zero_lamport_dead_bytes) as u64; + match self.shrink_ratio { - AccountShrinkThreshold::TotalSpace { shrink_ratio: _ } => { - (store.alive_bytes() as u64) < total_bytes - } + AccountShrinkThreshold::TotalSpace { shrink_ratio: _ } => alive_bytes < total_bytes, AccountShrinkThreshold::IndividualStore { shrink_ratio } => { - (store.alive_bytes() as f64 / total_bytes as f64) < shrink_ratio + (alive_bytes as f64 / total_bytes as f64) < shrink_ratio } } } diff --git a/accounts-db/src/accounts_file.rs b/accounts-db/src/accounts_file.rs index 3602f47c88fbec..4d3352662cc64a 100644 --- a/accounts-db/src/accounts_file.rs +++ b/accounts-db/src/accounts_file.rs @@ -98,6 +98,13 @@ impl AccountsFile { } } + pub(crate) fn dead_bytes_due_to_zero_lamport_single_ref(&self, count: usize) -> usize { + match self { + Self::AppendVec(av) => av.dead_bytes_due_to_zero_lamport_single_ref(count), + Self::TieredStorage(ts) => ts.dead_bytes_due_to_zero_lamport_single_ref(count), + } + } + pub fn flush(&self) -> Result<()> { match self { Self::AppendVec(av) => av.flush(), diff --git a/accounts-db/src/append_vec.rs b/accounts-db/src/append_vec.rs index 52f72477566459..c3df68d57674ae 100644 --- a/accounts-db/src/append_vec.rs +++ b/accounts-db/src/append_vec.rs @@ -409,6 +409,10 @@ impl AppendVec { } } + pub fn dead_bytes_due_to_zero_lamport_single_ref(&self, count: usize) -> usize { + aligned_stored_size(0) * count + } + pub fn flush(&self) -> Result<()> { match &self.backing { AppendVecFileBacking::Mmap(mmap_only) => { diff --git a/accounts-db/src/tiered_storage.rs b/accounts-db/src/tiered_storage.rs index 9df14bad1c93bf..d8f21a085e92e1 100644 --- a/accounts-db/src/tiered_storage.rs +++ b/accounts-db/src/tiered_storage.rs @@ -164,6 +164,11 @@ impl TieredStorage { self.reader() .map_or(MAX_TIERED_STORAGE_FILE_SIZE, |reader| reader.capacity()) } + + pub fn dead_bytes_due_to_zero_lamport_single_ref(&self, count: usize) -> usize { + // approximately 42 bytes per zero lamport account + count * 42 + } } #[cfg(test)] From a7fc9845971efc447101ef6591bce9cc39a78655 Mon Sep 17 00:00:00 2001 From: jeff washington Date: Fri, 6 Sep 2024 18:31:43 -0500 Subject: [PATCH 7/7] check ref count, too --- accounts-db/src/accounts_db.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index 00c4f6394f1962..f0c4acc226afff 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -4054,7 +4054,8 @@ impl AccountsDb { // If we are marking something dead, and the only remaining alive account is zero lamport, then make that zero lamport slot ready to be cleaned. // If that slot happens to only contain zero lamport accounts, the whole slot will go away - if slot_list.len() == 1 { + if slot_list.len() == 1 && ref_count == 2 { + // probably should have this check after we unref // should we also check for ref counts here? if let Some((slot_alive, acct_info)) = slot_list.first() { if acct_info.is_zero_lamport() && !acct_info.is_cached() {