From f247daaeed6054ad27415c8d46ae05420b52f0fb Mon Sep 17 00:00:00 2001 From: jeff washington Date: Fri, 14 Jun 2024 13:35:55 -0500 Subject: [PATCH 1/2] add ancient pack metrics --- accounts-db/src/accounts_db.rs | 12 ++++++ accounts-db/src/ancient_append_vecs.rs | 55 +++++++++++++++++++------- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index ba0b519e705b92..0b17c0972f1c43 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -1954,6 +1954,8 @@ pub(crate) struct ShrinkAncientStats { pub(crate) slots_considered: AtomicU64, pub(crate) ancient_scanned: AtomicU64, pub(crate) bytes_ancient_created: AtomicU64, + pub(crate) bytes_from_must_shrink: AtomicU64, + pub(crate) bytes_from_smallest_storages: AtomicU64, pub(crate) many_ref_slots_skipped: AtomicU64, pub(crate) slots_cannot_move_count: AtomicU64, pub(crate) many_refs_old_alive: AtomicU64, @@ -2246,6 +2248,16 @@ impl ShrinkAncientStats { self.bytes_ancient_created.swap(0, Ordering::Relaxed) as i64, i64 ), + ( + "bytes_from_must_shrink", + self.bytes_from_must_shrink.swap(0, Ordering::Relaxed) as i64, + i64 + ), + ( + "bytes_from_smallest_storages", + self.bytes_from_smallest_storages.swap(0, Ordering::Relaxed) as i64, + i64 + ), ( "many_ref_slots_skipped", self.many_ref_slots_skipped.swap(0, Ordering::Relaxed), diff --git a/accounts-db/src/ancient_append_vecs.rs b/accounts-db/src/ancient_append_vecs.rs index 392427724a5d4a..a65db7318f4698 100644 --- a/accounts-db/src/ancient_append_vecs.rs +++ b/accounts-db/src/ancient_append_vecs.rs @@ -8,8 +8,8 @@ use { account_storage::ShrinkInProgress, accounts_db::{ AccountFromStorage, AccountStorageEntry, AccountsDb, AliveAccounts, - GetUniqueAccountsResult, ShrinkCollect, ShrinkCollectAliveSeparatedByRefs, - ShrinkStatsSub, + GetUniqueAccountsResult, ShrinkAncientStats, ShrinkCollect, + ShrinkCollectAliveSeparatedByRefs, ShrinkStatsSub, }, accounts_file::AccountsFile, accounts_index::AccountsIndexScanResult, @@ -139,12 +139,16 @@ impl AncientSlotInfos { /// modify 'self' to contain only the slot infos for the slots that should be combined /// (and in this process effectively shrunk) - fn filter_ancient_slots(&mut self, tuning: &PackedAncientStorageTuning) { + fn filter_ancient_slots( + &mut self, + tuning: &PackedAncientStorageTuning, + stats: &ShrinkAncientStats, + ) { // figure out which slots to combine // 1. should_shrink: largest bytes saved above some cutoff of ratio self.choose_storages_to_shrink(tuning); // 2. smallest files so we get the largest number of files to remove - self.filter_by_smallest_capacity(tuning); + self.filter_by_smallest_capacity(tuning, stats); } // sort 'shrink_indexes' by most bytes saved, highest to lowest @@ -194,12 +198,18 @@ impl AncientSlotInfos { /// 'all_infos' are combined, the total number of storages <= 'max_storages' /// The idea is that 'all_infos' is sorted from smallest capacity to largest, /// but that isn't required for this function to be 'correct'. - fn truncate_to_max_storages(&mut self, tuning: &PackedAncientStorageTuning) { + fn truncate_to_max_storages( + &mut self, + tuning: &PackedAncientStorageTuning, + stats: &ShrinkAncientStats, + ) { // these indexes into 'all_infos' are useless once we truncate 'all_infos', so make sure they're cleared out to avoid any issues self.shrink_indexes.clear(); let total_storages = self.all_infos.len(); let mut cumulative_bytes = Saturating(0u64); let low_threshold = tuning.max_ancient_slots * 50 / 100; + let mut bytes_from_must_shrink = 0; + let mut bytes_from_smallest_storages = 0; for (i, info) in self.all_infos.iter().enumerate() { cumulative_bytes += info.alive_bytes; let ancient_storages_required = @@ -223,7 +233,18 @@ impl AncientSlotInfos { self.all_infos.truncate(i); break; } + if info.should_shrink { + bytes_from_must_shrink += info.alive_bytes; + } else { + bytes_from_smallest_storages += info.alive_bytes; + } } + stats + .bytes_from_must_shrink + .fetch_add(bytes_from_must_shrink, Ordering::Relaxed); + stats + .bytes_from_smallest_storages + .fetch_add(bytes_from_smallest_storages, Ordering::Relaxed); } /// remove entries from 'all_infos' such that combining @@ -233,7 +254,11 @@ impl AncientSlotInfos { /// Combining too many storages costs i/o and cpu so the goal is to find the sweet spot so /// that we make progress in cleaning/shrinking/combining but that we don't cause unnecessary /// churn. - fn filter_by_smallest_capacity(&mut self, tuning: &PackedAncientStorageTuning) { + fn filter_by_smallest_capacity( + &mut self, + tuning: &PackedAncientStorageTuning, + stats: &ShrinkAncientStats, + ) { let total_storages = self.all_infos.len(); if total_storages <= tuning.max_ancient_slots { // currently fewer storages than max, so nothing to shrink @@ -256,7 +281,7 @@ impl AncientSlotInfos { // remove any storages we don't need to combine this pass to achieve // # resulting storages <= 'max_storages' - self.truncate_to_max_storages(tuning); + self.truncate_to_max_storages(tuning, stats); } } @@ -470,7 +495,7 @@ impl AccountsDb { tuning.ideal_storage_size, ); - ancient_slot_infos.filter_ancient_slots(tuning); + ancient_slot_infos.filter_ancient_slots(tuning, &self.shrink_ancient_stats); ancient_slot_infos } @@ -2695,10 +2720,10 @@ pub mod tests { match method { TestSmallestCapacity::FilterAncientSlots => { infos.shrink_indexes.clear(); - infos.filter_ancient_slots(&tuning); + infos.filter_ancient_slots(&tuning, &ShrinkAncientStats::default()); } TestSmallestCapacity::FilterBySmallestCapacity => { - infos.filter_by_smallest_capacity(&tuning); + infos.filter_by_smallest_capacity(&tuning, &ShrinkAncientStats::default()); } } assert!(infos.all_infos.is_empty()); @@ -2741,11 +2766,11 @@ pub mod tests { }; match method { TestSmallestCapacity::FilterBySmallestCapacity => { - infos.filter_by_smallest_capacity(&tuning); + infos.filter_by_smallest_capacity(&tuning, &ShrinkAncientStats::default()); } TestSmallestCapacity::FilterAncientSlots => { infos.shrink_indexes.clear(); - infos.filter_ancient_slots(&tuning); + infos.filter_ancient_slots(&tuning, &ShrinkAncientStats::default()); } } assert_eq!( @@ -2862,9 +2887,9 @@ pub mod tests { fn test(filter: bool, infos: &mut AncientSlotInfos, tuning: &PackedAncientStorageTuning) { if filter { - infos.filter_by_smallest_capacity(tuning); + infos.filter_by_smallest_capacity(tuning, &ShrinkAncientStats::default()); } else { - infos.truncate_to_max_storages(tuning); + infos.truncate_to_max_storages(tuning, &ShrinkAncientStats::default()); } } @@ -3288,7 +3313,7 @@ pub mod tests { }; match method { TestShouldShrink::FilterAncientSlots => { - infos.filter_ancient_slots(&tuning); + infos.filter_ancient_slots(&tuning, &ShrinkAncientStats::default()); } TestShouldShrink::ClearShouldShrink => { infos.clear_should_shrink_after_cutoff(&tuning); From c5720543d1deb01dda528e7f04170e1b3646b3fa Mon Sep 17 00:00:00 2001 From: brooks Date: Tue, 18 Jun 2024 12:38:54 -0400 Subject: [PATCH 2/2] add missing params --- accounts-db/src/ancient_append_vecs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/accounts-db/src/ancient_append_vecs.rs b/accounts-db/src/ancient_append_vecs.rs index a65db7318f4698..916c475bece372 100644 --- a/accounts-db/src/ancient_append_vecs.rs +++ b/accounts-db/src/ancient_append_vecs.rs @@ -2821,7 +2821,7 @@ pub mod tests { // shuffle the infos so they actually need to be sorted infos.all_infos.shuffle(&mut thread_rng()); - infos.filter_by_smallest_capacity(&tuning); + infos.filter_by_smallest_capacity(&tuning, &ShrinkAncientStats::default()); infos .all_infos @@ -2866,7 +2866,7 @@ pub mod tests { // shuffle the infos so they actually need to be sorted infos.all_infos.shuffle(&mut thread_rng()); - infos.filter_by_smallest_capacity(&tuning); + infos.filter_by_smallest_capacity(&tuning, &ShrinkAncientStats::default()); infos .all_infos