diff --git a/accounts-db/src/accounts_db.rs b/accounts-db/src/accounts_db.rs index fab7eb290f7323..a6b7feea1e2ab8 100644 --- a/accounts-db/src/accounts_db.rs +++ b/accounts-db/src/accounts_db.rs @@ -2438,28 +2438,11 @@ impl AccountsDb { is_startup: bool, timings: &mut CleanKeyTimings, epoch_schedule: &EpochSchedule, - old_storages_policy: OldStoragesPolicy, ) -> CleaningCandidates { let oldest_non_ancient_slot = self.get_oldest_non_ancient_slot(epoch_schedule); let mut dirty_store_processing_time = Measure::start("dirty_store_processing"); let max_root_inclusive = self.accounts_index.max_root_inclusive(); let max_slot_inclusive = max_clean_root_inclusive.unwrap_or(max_root_inclusive); - - if old_storages_policy == OldStoragesPolicy::Clean { - let slot_one_epoch_old = - max_root_inclusive.saturating_sub(epoch_schedule.slots_per_epoch); - // do nothing special for these 100 old storages that will likely get cleaned up shortly - let acceptable_straggler_slot_count = 100; - let old_slot_cutoff = - slot_one_epoch_old.saturating_sub(acceptable_straggler_slot_count); - let (old_storages, old_slots) = self.get_storages(..old_slot_cutoff); - let num_old_storages = old_storages.len(); - for (old_slot, old_storage) in std::iter::zip(old_slots, old_storages) { - self.dirty_stores.entry(old_slot).or_insert(old_storage); - } - info!("Marked {num_old_storages} old storages as dirty"); - } - let mut dirty_stores = Vec::with_capacity(self.dirty_stores.len()); // find the oldest dirty slot // we'll add logging if that append vec cannot be marked dead @@ -2571,16 +2554,7 @@ impl AccountsDb { /// Call clean_accounts() with the common parameters that tests/benches use. pub fn clean_accounts_for_tests(&self) { - self.clean_accounts( - None, - false, - &EpochSchedule::default(), - if self.ancient_append_vec_offset.is_some() { - OldStoragesPolicy::Leave - } else { - OldStoragesPolicy::Clean - }, - ) + self.clean_accounts(None, false, &EpochSchedule::default()) } /// called with cli argument to verify refcounts are correct on all accounts @@ -2689,7 +2663,6 @@ impl AccountsDb { max_clean_root_inclusive: Option, is_startup: bool, epoch_schedule: &EpochSchedule, - old_storages_policy: OldStoragesPolicy, ) { if self.exhaustively_verify_refcounts { //at startup use all cores to verify refcounts @@ -2722,7 +2695,6 @@ impl AccountsDb { is_startup, &mut key_timings, epoch_schedule, - old_storages_policy, ); measure_construct_candidates.stop(); drop(active_guard); @@ -4318,15 +4290,7 @@ impl AccountsDb { let maybe_clean = || { if self.dirty_stores.len() > DIRTY_STORES_CLEANING_THRESHOLD { let latest_full_snapshot_slot = self.latest_full_snapshot_slot(); - self.clean_accounts( - latest_full_snapshot_slot, - is_startup, - epoch_schedule, - // Leave any old storages alone for now. Once the validator is running - // normal, calls to clean_accounts() will have the correct policy based - // on if ancient storages are enabled or not. - OldStoragesPolicy::Leave, - ); + self.clean_accounts(latest_full_snapshot_slot, is_startup, epoch_schedule); } }; @@ -8826,20 +8790,6 @@ pub(crate) enum UpdateIndexThreadSelection { PoolWithThreshold, } -/// How should old storages be handled in clean_accounts()? -#[derive(Debug, Copy, Clone, Eq, PartialEq)] -pub enum OldStoragesPolicy { - /// Clean all old storages, even if they were not explictly marked as dirty. - /// - /// This is the default behavior when not skipping rewrites. - Clean, - /// Leave all old storages. - /// - /// When skipping rewrites, we intentionally will have ancient storages. - /// Do not clean them up automatically in clean_accounts(). - Leave, -} - // These functions/fields are only usable from a dev context (i.e. tests and benches) #[cfg(feature = "dev-context-only-utils")] impl AccountStorageEntry { diff --git a/accounts-db/src/accounts_db/tests.rs b/accounts-db/src/accounts_db/tests.rs index 37d47e2460938b..6a98454cf8f268 100644 --- a/accounts-db/src/accounts_db/tests.rs +++ b/accounts-db/src/accounts_db/tests.rs @@ -1709,23 +1709,13 @@ fn test_clean_max_slot_zero_lamport_account() { // updates in later slots in slot 1 assert_eq!(accounts.alive_account_count_in_slot(0), 1); assert_eq!(accounts.alive_account_count_in_slot(1), 1); - accounts.clean_accounts( - Some(0), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + accounts.clean_accounts(Some(0), false, &EpochSchedule::default()); assert_eq!(accounts.alive_account_count_in_slot(0), 1); assert_eq!(accounts.alive_account_count_in_slot(1), 1); assert!(accounts.accounts_index.contains_with(&pubkey, None, None)); // Now the account can be cleaned up - accounts.clean_accounts( - Some(1), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + accounts.clean_accounts(Some(1), false, &EpochSchedule::default()); assert_eq!(accounts.alive_account_count_in_slot(0), 0); assert_eq!(accounts.alive_account_count_in_slot(1), 0); @@ -3194,12 +3184,7 @@ fn test_zero_lamport_new_root_not_cleaned() { db.add_root_and_flush_write_cache(1); // Only clean zero lamport accounts up to slot 0 - db.clean_accounts( - Some(0), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + db.clean_accounts(Some(0), false, &EpochSchedule::default()); // Should still be able to find zero lamport account in slot 1 assert_eq!( @@ -4404,12 +4389,7 @@ fn run_test_shrink_unref(do_intra_cache_clean: bool) { db.calculate_accounts_delta_hash(1); // Clean to remove outdated entry from slot 0 - db.clean_accounts( - Some(1), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + db.clean_accounts(Some(1), false, &EpochSchedule::default()); // Shrink Slot 0 { @@ -4428,12 +4408,7 @@ fn run_test_shrink_unref(do_intra_cache_clean: bool) { // Should be one store before clean for slot 0 db.get_and_assert_single_storage(0); db.calculate_accounts_delta_hash(2); - db.clean_accounts( - Some(2), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + db.clean_accounts(Some(2), false, &EpochSchedule::default()); // No stores should exist for slot 0 after clean assert_no_storages_at_slot(&db, 0); @@ -4478,7 +4453,7 @@ fn test_clean_drop_dead_zero_lamport_single_ref_accounts() { accounts_db.calculate_accounts_delta_hash(1); // run clean - accounts_db.clean_accounts(Some(1), false, &epoch_schedule, OldStoragesPolicy::Leave); + accounts_db.clean_accounts(Some(1), false, &epoch_schedule); // After clean, both slot0 and slot1 should be marked dead and dropped // from the store map. @@ -4510,12 +4485,7 @@ fn test_clean_drop_dead_storage_handle_zero_lamport_single_ref_accounts() { // Clean should mark slot 0 dead and drop it. During the dropping, it // will find that slot 1 has a single ref zero accounts and mark it. - db.clean_accounts( - Some(1), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + db.clean_accounts(Some(1), false, &EpochSchedule::default()); // Assert that after clean, slot 0 is dropped. assert!(db.storage.get_slot_storage_entry(0).is_none()); @@ -4556,12 +4526,7 @@ fn test_shrink_unref_handle_zero_lamport_single_ref_accounts() { db.calculate_accounts_delta_hash(1); // Clean to remove outdated entry from slot 0 - db.clean_accounts( - Some(1), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + db.clean_accounts(Some(1), false, &EpochSchedule::default()); // Shrink Slot 0 { @@ -4595,12 +4560,7 @@ fn test_shrink_unref_handle_zero_lamport_single_ref_accounts() { db.get_and_assert_single_storage(0); db.get_and_assert_single_storage(1); db.calculate_accounts_delta_hash(2); - db.clean_accounts( - Some(2), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + db.clean_accounts(Some(2), false, &EpochSchedule::default()); // No stores should exist for slot 0 after clean assert_no_storages_at_slot(&db, 0); @@ -5414,30 +5374,15 @@ define_accounts_db_test!( assert_eq!(accounts_db.ref_count_for_pubkey(&pubkey), 3); accounts_db.set_latest_full_snapshot_slot(slot2); - accounts_db.clean_accounts( - Some(slot2), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + accounts_db.clean_accounts(Some(slot2), false, &EpochSchedule::default()); assert_eq!(accounts_db.ref_count_for_pubkey(&pubkey), 2); accounts_db.set_latest_full_snapshot_slot(slot2); - accounts_db.clean_accounts( - None, - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + accounts_db.clean_accounts(None, false, &EpochSchedule::default()); assert_eq!(accounts_db.ref_count_for_pubkey(&pubkey), 1); accounts_db.set_latest_full_snapshot_slot(slot3); - accounts_db.clean_accounts( - None, - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + accounts_db.clean_accounts(None, false, &EpochSchedule::default()); assert_eq!(accounts_db.ref_count_for_pubkey(&pubkey), 0); } ); @@ -7169,12 +7114,7 @@ define_accounts_db_test!(test_calculate_incremental_accounts_hash, |accounts_db| // calculate the full accounts hash let full_accounts_hash = { - accounts_db.clean_accounts( - Some(slot - 1), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + accounts_db.clean_accounts(Some(slot - 1), false, &EpochSchedule::default()); let (storages, _) = accounts_db.get_storages(..=slot); let storages = SortedStorages::new(&storages); accounts_db.calculate_accounts_hash( @@ -7240,12 +7180,7 @@ define_accounts_db_test!(test_calculate_incremental_accounts_hash, |accounts_db| // calculate the incremental accounts hash let incremental_accounts_hash = { accounts_db.set_latest_full_snapshot_slot(full_accounts_hash_slot); - accounts_db.clean_accounts( - Some(slot - 1), - false, - &EpochSchedule::default(), - OldStoragesPolicy::Leave, - ); + accounts_db.clean_accounts(Some(slot - 1), false, &EpochSchedule::default()); let (storages, _) = accounts_db.get_storages(full_accounts_hash_slot + 1..=slot); let storages = SortedStorages::new(&storages); accounts_db.calculate_incremental_accounts_hash( diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index 93826c5ff67f0d..29cd48b79c823c 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -678,7 +678,6 @@ impl AccountsBackgroundService { Some(max_clean_slot_inclusive), false, bank.epoch_schedule(), - bank.clean_accounts_old_storages_policy(), ); last_cleaned_slot = max_clean_slot_inclusive; previous_clean_time = Instant::now(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 1d71b9c0d5e2c6..2c6d526b0b8da0 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -79,8 +79,7 @@ use { accounts::{AccountAddressFilter, Accounts, PubkeyAccountSlot}, accounts_db::{ AccountStorageEntry, AccountsDb, AccountsDbConfig, CalcAccountsHashDataSource, - DuplicatesLtHash, OldStoragesPolicy, PubkeyHashAccount, - VerifyAccountsHashAndLamportsConfig, + DuplicatesLtHash, PubkeyHashAccount, VerifyAccountsHashAndLamportsConfig, }, accounts_hash::{ AccountHash, AccountsHash, AccountsLtHash, CalcAccountsHashConfig, HashStats, @@ -6048,7 +6047,6 @@ impl Bank { Some(latest_full_snapshot_slot), true, self.epoch_schedule(), - self.clean_accounts_old_storages_policy(), ); info!("Cleaning... Done."); } else { @@ -6395,7 +6393,6 @@ impl Bank { Some(highest_slot_to_clean), false, self.epoch_schedule(), - self.clean_accounts_old_storages_policy(), ); } @@ -6433,15 +6430,6 @@ impl Bank { can_skip_rewrites || test_skip_rewrites_but_include_in_bank_hash } - /// Returns how clean_accounts() should handle old storages - pub fn clean_accounts_old_storages_policy(&self) -> OldStoragesPolicy { - if self.are_ancient_storages_enabled() { - OldStoragesPolicy::Leave - } else { - OldStoragesPolicy::Clean - } - } - pub fn read_cost_tracker(&self) -> LockResult> { self.cost_tracker.read() }