From 8d98589c8527867464d44920e590c34abc172b79 Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" Date: Mon, 8 May 2023 12:50:27 -0500 Subject: [PATCH] remove unused historical_roots (#31539) --- runtime/src/accounts_db.rs | 65 --------- runtime/src/accounts_index.rs | 214 ---------------------------- runtime/src/serde_snapshot.rs | 32 +---- runtime/src/serde_snapshot/newer.rs | 9 +- runtime/src/serde_snapshot/tests.rs | 32 ----- 5 files changed, 3 insertions(+), 349 deletions(-) diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index e010baab95..e726f6d3f2 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -1811,7 +1811,6 @@ struct FlushStats { #[derive(Debug, Default)] struct LatestAccountsIndexRootsStats { roots_len: AtomicUsize, - historical_roots_len: AtomicUsize, uncleaned_roots_len: AtomicUsize, previous_uncleaned_roots_len: AtomicUsize, roots_range: AtomicU64, @@ -1833,9 +1832,6 @@ impl LatestAccountsIndexRootsStats { self.previous_uncleaned_roots_len .store(value, Ordering::Relaxed); } - if let Some(value) = accounts_index_roots_stats.historical_roots_len { - self.historical_roots_len.store(value, Ordering::Relaxed); - } if let Some(value) = accounts_index_roots_stats.roots_range { self.roots_range.store(value, Ordering::Relaxed); } @@ -1865,11 +1861,6 @@ impl LatestAccountsIndexRootsStats { self.roots_len.load(Ordering::Relaxed) as i64, i64 ), - ( - "historical_roots_len", - self.historical_roots_len.load(Ordering::Relaxed) as i64, - i64 - ), ( "uncleaned_roots_len", self.uncleaned_roots_len.load(Ordering::Relaxed) as i64, @@ -2931,12 +2922,6 @@ impl AccountsDb { *accounts_hash_complete_oldest_non_ancient_slot, one_epoch_old_slot, ); - - let accounts_hash_complete_oldest_non_ancient_slot = - *accounts_hash_complete_oldest_non_ancient_slot; - - // now that accounts hash calculation is complete, we can remove old historical roots - self.remove_old_historical_roots(accounts_hash_complete_oldest_non_ancient_slot); } /// get the oldest slot that is within one epoch of the highest slot that has been used for hash calculation. @@ -6888,11 +6873,6 @@ impl AccountsDb { ); } - /// find slot >= 'slot' which is a root or in 'ancestors' - pub fn find_unskipped_slot(&self, slot: Slot, ancestors: Option<&Ancestors>) -> Option { - self.accounts_index.get_next_original_root(slot, ancestors) - } - pub fn checked_iterative_sum_for_capitalization(total_cap: u64, new_cap: u64) -> u64 { let new_total = total_cap as u128 + new_cap as u128; AccountsHasher::checked_cast_for_capitalization(new_total) @@ -7727,32 +7707,6 @@ impl AccountsDb { result } - /// return alive roots to retain, even though they are ancient - fn calc_alive_ancient_historical_roots(&self, min_root: Slot) -> HashSet { - let mut ancient_alive_roots = HashSet::default(); - { - let all_roots = self.accounts_index.roots_tracker.read().unwrap(); - - if let Some(min) = all_roots.historical_roots.min() { - for slot in min..min_root { - if all_roots.alive_roots.contains(&slot) { - // there was a storage for this root, so it counts as a root - ancient_alive_roots.insert(slot); - } - } - } - } - ancient_alive_roots - } - - /// get rid of historical roots that are older than 'min_root'. - /// These will be older than an epoch from a current root. - fn remove_old_historical_roots(&self, min_root: Slot) { - let alive_roots = self.calc_alive_ancient_historical_roots(min_root); - self.accounts_index - .remove_old_historical_roots(min_root, &alive_roots); - } - /// Verify accounts hash at startup (or tests) /// /// Calculate accounts hash(es) and compare them to the values set at startup. @@ -16339,25 +16293,6 @@ pub mod tests { } } - #[test] - fn test_calc_alive_ancient_historical_roots() { - let db = AccountsDb::new_single_for_tests(); - let min_root = 0; - let result = db.calc_alive_ancient_historical_roots(min_root); - assert!(result.is_empty()); - for extra in 1..3 { - let result = db.calc_alive_ancient_historical_roots(extra); - assert_eq!(result, HashSet::default(), "extra: {extra}"); - } - - let extra = 3; - let active_root = 2; - db.accounts_index.add_root(active_root); - let result = db.calc_alive_ancient_historical_roots(extra); - let expected_alive_roots = [active_root].into_iter().collect(); - assert_eq!(result, expected_alive_roots, "extra: {extra}"); - } - impl AccountsDb { /// useful to adapt tests written prior to introduction of the write cache /// to use the write cache diff --git a/runtime/src/accounts_index.rs b/runtime/src/accounts_index.rs index 42fcc21930..a8380c3ba7 100644 --- a/runtime/src/accounts_index.rs +++ b/runtime/src/accounts_index.rs @@ -450,12 +450,6 @@ pub struct RootsTracker { /// Updated every time we add a new root or clean/shrink an append vec into irrelevancy. /// Range is approximately the last N slots where N is # slots per epoch. pub(crate) alive_roots: RollingBitField, - /// Set of roots that are roots now or were roots at one point in time. - /// Range is approximately the last N slots where N is # slots per epoch. - /// A root could remain here if all entries in the append vec at that root are cleaned/shrunk and there are no - /// more entries for that slot. 'alive_roots' will no longer contain such roots. - /// This is a superset of 'alive_roots' - pub(crate) historical_roots: RollingBitField, uncleaned_roots: HashSet, previous_uncleaned_roots: HashSet, } @@ -473,7 +467,6 @@ impl RootsTracker { pub fn new(max_width: u64) -> Self { Self { alive_roots: RollingBitField::new(max_width), - historical_roots: RollingBitField::new(max_width), uncleaned_roots: HashSet::new(), previous_uncleaned_roots: HashSet::new(), } @@ -490,7 +483,6 @@ pub struct AccountsIndexRootsStats { pub uncleaned_roots_len: Option, pub previous_uncleaned_roots_len: Option, pub roots_range: Option, - pub historical_roots_len: Option, pub rooted_cleaned_count: usize, pub unrooted_cleaned_count: usize, pub clean_unref_from_storage_us: u64, @@ -1876,7 +1868,6 @@ impl + Into> AccountsIndex { assert!(slot >= w_roots_tracker.alive_roots.max_inclusive()); // 'slot' is a root, so it is both 'root' and 'original' w_roots_tracker.alive_roots.insert(slot); - w_roots_tracker.historical_roots.insert(slot); } pub fn add_uncleaned_roots(&self, roots: I) @@ -1895,52 +1886,6 @@ impl + Into> AccountsIndex { .max_inclusive() } - /// return the lowest original root >= slot, including historical_roots and ancestors - pub fn get_next_original_root( - &self, - slot: Slot, - ancestors: Option<&Ancestors>, - ) -> Option { - { - let roots_tracker = self.roots_tracker.read().unwrap(); - for root in slot..roots_tracker.historical_roots.max_exclusive() { - if roots_tracker.historical_roots.contains(&root) { - return Some(root); - } - } - } - // ancestors are higher than roots, so look for roots first - if let Some(ancestors) = ancestors { - let min = std::cmp::max(slot, ancestors.min_slot()); - for root in min..=ancestors.max_slot() { - if ancestors.contains_key(&root) { - return Some(root); - } - } - } - None - } - - /// roots are inserted into 'historical_roots' and 'roots' as a new root is made. - /// roots are removed form 'roots' as all entries in the append vec become outdated. - /// This function exists to clean older entries from 'historical_roots'. - /// all roots < 'oldest_slot_to_keep' are removed from 'historical_roots'. - pub fn remove_old_historical_roots(&self, oldest_slot_to_keep: Slot, keep: &HashSet) { - let mut roots = self - .roots_tracker - .read() - .unwrap() - .historical_roots - .get_all_less_than(oldest_slot_to_keep); - roots.retain(|root| !keep.contains(root)); - if !roots.is_empty() { - let mut w_roots_tracker = self.roots_tracker.write().unwrap(); - roots.into_iter().for_each(|root| { - w_roots_tracker.historical_roots.remove(&root); - }); - } - } - /// Remove the slot when the storage for the slot is freed /// Accounts no longer reference this slot. /// return true if slot was a root @@ -1972,7 +1917,6 @@ impl + Into> AccountsIndex { stats.previous_uncleaned_roots_len = Some(w_roots_tracker.previous_uncleaned_roots.len()); stats.roots_range = Some(w_roots_tracker.alive_roots.range_width()); - stats.historical_roots_len = Some(w_roots_tracker.historical_roots.len()); drop(w_roots_tracker); self.roots_removed.fetch_add(1, Ordering::Relaxed); true @@ -2178,164 +2122,6 @@ pub mod tests { } } - #[test] - fn test_get_next_original_root() { - let ancestors = None; - let index = AccountsIndex::::default_for_tests(); - for slot in 0..2 { - assert_eq!(index.get_next_original_root(slot, ancestors), None); - } - // roots are now [1]. 0 and 1 both return 1 - index.add_root(1); - for slot in 0..2 { - assert_eq!(index.get_next_original_root(slot, ancestors), Some(1)); - } - assert_eq!(index.get_next_original_root(2, ancestors), None); // no roots after 1, so asking for root >= 2 is None - - // roots are now [1, 3]. 0 and 1 both return 1. 2 and 3 both return 3 - index.add_root(3); - for slot in 0..2 { - assert_eq!(index.get_next_original_root(slot, ancestors), Some(1)); - } - for slot in 2..4 { - assert_eq!(index.get_next_original_root(slot, ancestors), Some(3)); - } - assert_eq!(index.get_next_original_root(4, ancestors), None); // no roots after 3, so asking for root >= 4 is None - } - - #[test] - fn test_get_next_original_root_ancestors() { - let orig_ancestors = Ancestors::default(); - let ancestors = Some(&orig_ancestors); - let index = AccountsIndex::::default_for_tests(); - for slot in 0..2 { - assert_eq!(index.get_next_original_root(slot, ancestors), None); - } - // ancestors are now [1]. 0 and 1 both return 1 - let orig_ancestors = Ancestors::from(vec![1]); - let ancestors = Some(&orig_ancestors); - for slot in 0..2 { - assert_eq!(index.get_next_original_root(slot, ancestors), Some(1)); - } - assert_eq!(index.get_next_original_root(2, ancestors), None); // no roots after 1, so asking for root >= 2 is None - - // ancestors are now [1, 3]. 0 and 1 both return 1. 2 and 3 both return 3 - let orig_ancestors = Ancestors::from(vec![1, 3]); - let ancestors = Some(&orig_ancestors); - for slot in 0..2 { - assert_eq!(index.get_next_original_root(slot, ancestors), Some(1)); - } - for slot in 2..4 { - assert_eq!(index.get_next_original_root(slot, ancestors), Some(3)); - } - assert_eq!(index.get_next_original_root(4, ancestors), None); // no roots after 3, so asking for root >= 4 is None - } - - #[test] - fn test_get_next_original_root_roots_and_ancestors() { - let orig_ancestors = Ancestors::default(); - let ancestors = Some(&orig_ancestors); - let index = AccountsIndex::::default_for_tests(); - for slot in 0..2 { - assert_eq!(index.get_next_original_root(slot, ancestors), None); - } - // roots are now [1]. 0 and 1 both return 1 - index.add_root(1); - for slot in 0..2 { - assert_eq!(index.get_next_original_root(slot, ancestors), Some(1)); - } - assert_eq!(index.get_next_original_root(2, ancestors), None); // no roots after 1, so asking for root >= 2 is None - - // roots are now [1] and ancestors are now [3]. 0 and 1 both return 1. 2 and 3 both return 3 - let orig_ancestors = Ancestors::from(vec![3]); - let ancestors = Some(&orig_ancestors); - for slot in 0..2 { - assert_eq!(index.get_next_original_root(slot, ancestors), Some(1)); - } - for slot in 2..4 { - assert_eq!(index.get_next_original_root(slot, ancestors), Some(3)); - } - assert_eq!(index.get_next_original_root(4, ancestors), None); // no roots after 3, so asking for root >= 4 is None - } - - #[test] - fn test_remove_old_historical_roots() { - let index = AccountsIndex::::default_for_tests(); - index.add_root(1); - index.add_root(2); - assert_eq!( - index - .roots_tracker - .read() - .unwrap() - .historical_roots - .get_all(), - vec![1, 2] - ); - let empty_hash_set = HashSet::default(); - index.remove_old_historical_roots(2, &empty_hash_set); - assert_eq!( - index - .roots_tracker - .read() - .unwrap() - .historical_roots - .get_all(), - vec![2] - ); - index.remove_old_historical_roots(3, &empty_hash_set); - assert!( - index - .roots_tracker - .read() - .unwrap() - .historical_roots - .is_empty(), - "{:?}", - index - .roots_tracker - .read() - .unwrap() - .historical_roots - .get_all() - ); - - // now use 'keep' - let index = AccountsIndex::::default_for_tests(); - index.add_root(1); - index.add_root(2); - let hash_set_1 = vec![1].into_iter().collect(); - assert_eq!( - index - .roots_tracker - .read() - .unwrap() - .historical_roots - .get_all(), - vec![1, 2] - ); - index.remove_old_historical_roots(2, &hash_set_1); - assert_eq!( - index - .roots_tracker - .read() - .unwrap() - .historical_roots - .get_all(), - vec![1, 2] - ); - index.remove_old_historical_roots(3, &hash_set_1); - assert_eq!( - index - .roots_tracker - .read() - .unwrap() - .historical_roots - .get_all(), - vec![1] - ); - } - const COLLECT_ALL_UNSORTED_FALSE: bool = false; #[test] diff --git a/runtime/src/serde_snapshot.rs b/runtime/src/serde_snapshot.rs index 4cd1a7453c..59e603d6d8 100644 --- a/runtime/src/serde_snapshot.rs +++ b/runtime/src/serde_snapshot.rs @@ -818,8 +818,8 @@ where snapshot_version, snapshot_slot, snapshot_bank_hash_info, - snapshot_historical_roots, - snapshot_historical_roots_with_hash, + _snapshot_historical_roots, + _snapshot_historical_roots_with_hash, ) = snapshot_accounts_db_fields.collapse_into()?; // Ensure all account paths exist @@ -828,12 +828,6 @@ where .unwrap_or_else(|err| panic!("Failed to create directory {}: {}", path.display(), err)); } - reconstruct_historical_roots( - &accounts_db, - snapshot_historical_roots, - snapshot_historical_roots_with_hash, - ); - let StorageAndNextAppendVecId { storage, next_append_vec_id, @@ -914,25 +908,3 @@ where ReconstructedAccountsDbInfo { accounts_data_len }, )) } - -/// populate 'historical_roots' from 'snapshot_historical_roots' and 'snapshot_historical_roots_with_hash' -fn reconstruct_historical_roots( - accounts_db: &AccountsDb, - mut snapshot_historical_roots: Vec, - snapshot_historical_roots_with_hash: Vec<(Slot, Hash)>, -) { - // inflate 'historical_roots' - // inserting into 'historical_roots' needs to be in order - // combine the slots into 1 vec, then sort - // dups are ok - snapshot_historical_roots.extend( - snapshot_historical_roots_with_hash - .into_iter() - .map(|(root, _)| root), - ); - snapshot_historical_roots.sort_unstable(); - let mut roots_tracker = accounts_db.accounts_index.roots_tracker.write().unwrap(); - snapshot_historical_roots.into_iter().for_each(|root| { - roots_tracker.historical_roots.insert(root); - }); -} diff --git a/runtime/src/serde_snapshot/newer.rs b/runtime/src/serde_snapshot/newer.rs index fd9ed5e1dc..e0dc09b58a 100644 --- a/runtime/src/serde_snapshot/newer.rs +++ b/runtime/src/serde_snapshot/newer.rs @@ -294,14 +294,7 @@ impl<'a> TypeContext<'a> for Context { stats, }; - let historical_roots = serializable_db - .accounts_db - .accounts_index - .roots_tracker - .read() - .unwrap() - .historical_roots - .get_all(); + let historical_roots = Vec::::default(); let historical_roots_with_hash = Vec::<(Slot, Hash)>::default(); let mut serialize_account_storage_timer = Measure::start("serialize_account_storage_ms"); diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index 43c09f1fca..14acec1e04 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -752,35 +752,3 @@ mod test_bank_serialize { .serialize(s) } } - -#[test] -fn test_reconstruct_historical_roots() { - { - let db = AccountsDb::default_for_tests(); - let historical_roots = vec![]; - let historical_roots_with_hash = vec![]; - reconstruct_historical_roots(&db, historical_roots, historical_roots_with_hash); - let roots_tracker = db.accounts_index.roots_tracker.read().unwrap(); - assert!(roots_tracker.historical_roots.is_empty()); - } - - { - let db = AccountsDb::default_for_tests(); - let historical_roots = vec![1]; - let historical_roots_with_hash = vec![(0, Hash::default())]; - reconstruct_historical_roots(&db, historical_roots, historical_roots_with_hash); - let roots_tracker = db.accounts_index.roots_tracker.read().unwrap(); - assert_eq!(roots_tracker.historical_roots.get_all(), vec![0, 1]); - } - { - let db = AccountsDb::default_for_tests(); - let historical_roots = vec![2, 1]; - let historical_roots_with_hash = vec![0, 5] - .into_iter() - .map(|slot| (slot, Hash::default())) - .collect(); - reconstruct_historical_roots(&db, historical_roots, historical_roots_with_hash); - let roots_tracker = db.accounts_index.roots_tracker.read().unwrap(); - assert_eq!(roots_tracker.historical_roots.get_all(), vec![0, 1, 2, 5]); - } -}