From 17a99d98dd014d9a1ac690368e2f71cbd946047b Mon Sep 17 00:00:00 2001 From: "Jeff Washington (jwash)" Date: Wed, 6 Jul 2022 11:32:45 -0500 Subject: [PATCH] =?UTF-8?q?Revert=20"avoid=20adding=20to=20'uncleaned=5Fro?= =?UTF-8?q?ots'=20when=20generating=20index=20and=20c=E2=80=A6=20(#26441)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert "avoid adding to 'uncleaned_roots' when generating index and caller passes accounts-db-skip-shrink (#25936)" This reverts commit e24cc537a4b577dcd001f008e3795c1040a7d8dd. --- runtime/src/accounts_db.rs | 8 +------- runtime/src/bank.rs | 18 +++--------------- runtime/src/serde_snapshot.rs | 6 ------ runtime/src/serde_snapshot/tests.rs | 4 ---- runtime/src/snapshot_utils.rs | 8 +------- 5 files changed, 5 insertions(+), 39 deletions(-) diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index feca3bd597..3f4e7dc17b 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -8142,7 +8142,6 @@ impl AccountsDb { limit_load_slot_count_from_snapshot: Option, verify: bool, genesis_config: &GenesisConfig, - accounts_db_skip_shrink: bool, ) -> IndexGenerationInfo { let mut slots = self.storage.all_slots(); #[allow(clippy::stable_sort_primitive)] @@ -8345,12 +8344,7 @@ impl AccountsDb { if pass == 0 { // Need to add these last, otherwise older updates will be cleaned for slot in &slots { - // passing 'false' to 'add_root' causes all slots to be added to 'uncleaned_slots' - // passing 'true' to 'add_root' does NOT add all slots to 'uncleaned_slots' - // if we are skipping shrink, this potentially massive amount of work is never processed at startup, when all threads can be used. - // This causes failures such as oom during the first bg clean, which is expecting to work in 'normal' operating circumstances. - // So, don't add all slots to 'uncleaned_slots' here since by requesting to skip clean and shrink, caller is expecting the starting snapshot to be reasonable. - self.accounts_index.add_root(*slot, accounts_db_skip_shrink); + self.accounts_index.add_root(*slot, false); } self.set_storage_count_and_alive_bytes(storage_info, &mut timings); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d5265e6f21..4930d16fe0 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -6921,21 +6921,9 @@ impl Bank { last_full_snapshot_slot: Option, ) -> bool { let mut clean_time = Measure::start("clean"); - if !accounts_db_skip_shrink { - if self.slot() > 0 { - info!("cleaning.."); - self.clean_accounts(true, true, last_full_snapshot_slot); - } - } else { - // if we are skipping shrink, there should be no uncleaned_roots deferred to later - assert_eq!( - self.rc - .accounts - .accounts_db - .accounts_index - .uncleaned_roots_len(), - 0 - ); + if !accounts_db_skip_shrink && self.slot() > 0 { + info!("cleaning.."); + self.clean_accounts(true, true, last_full_snapshot_slot); } clean_time.stop(); diff --git a/runtime/src/serde_snapshot.rs b/runtime/src/serde_snapshot.rs index b66eb9c5e7..b2586307ec 100644 --- a/runtime/src/serde_snapshot.rs +++ b/runtime/src/serde_snapshot.rs @@ -242,7 +242,6 @@ pub(crate) fn bank_from_streams( verify_index: bool, accounts_db_config: Option, accounts_update_notifier: Option, - accounts_db_skip_shrink: bool, ) -> std::result::Result where R: Read, @@ -294,7 +293,6 @@ where verify_index, accounts_db_config, accounts_update_notifier, - accounts_db_skip_shrink, ) } @@ -475,7 +473,6 @@ fn reconstruct_bank_from_fields( verify_index: bool, accounts_db_config: Option, accounts_update_notifier: Option, - accounts_db_skip_shrink: bool, ) -> Result where E: SerializableStorage + std::marker::Sync, @@ -492,7 +489,6 @@ where verify_index, accounts_db_config, accounts_update_notifier, - accounts_db_skip_shrink, )?; let bank_rc = BankRc::new(Accounts::new_empty(accounts_db), bank_fields.slot); @@ -552,7 +548,6 @@ fn reconstruct_accountsdb_from_fields( verify_index: bool, accounts_db_config: Option, accounts_update_notifier: Option, - accounts_db_skip_shrink: bool, ) -> Result<(AccountsDb, ReconstructedAccountsDbInfo), Error> where E: SerializableStorage + std::marker::Sync, @@ -702,7 +697,6 @@ where limit_load_slot_count_from_snapshot, verify_index, genesis_config, - accounts_db_skip_shrink, ); accounts_db.maybe_add_filler_accounts( diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index ab80c166c3..39a7fe7fc4 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -90,7 +90,6 @@ where false, Some(crate::accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING), None, - false, ) .map(|(accounts_db, _)| accounts_db) } @@ -303,7 +302,6 @@ fn test_bank_serialize_style( false, Some(crate::accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING), None, - false, ) .unwrap(); dbank.status_cache = Arc::new(RwLock::new(status_cache)); @@ -419,7 +417,6 @@ fn test_extra_fields_eof() { false, Some(crate::accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING), None, - false, ) .unwrap(); @@ -541,7 +538,6 @@ fn test_blank_extra_fields() { false, Some(crate::accounts_db::ACCOUNTS_DB_CONFIG_FOR_TESTING), None, - false, ) .unwrap(); diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index e258403768..e8bbcb13ad 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -821,9 +821,6 @@ pub fn bank_from_snapshot_archives( incremental_snapshot_archive_info, )?; - let accounts_db_skip_shrink = - accounts_db_skip_shrink || !full_snapshot_archive_info.is_remote(); - let parallel_divisions = std::cmp::min( PARALLEL_UNTAR_READERS_DEFAULT, std::cmp::max(1, num_cpus::get() / 4), @@ -882,7 +879,6 @@ pub fn bank_from_snapshot_archives( verify_index, accounts_db_config, accounts_update_notifier, - accounts_db_skip_shrink, )?; measure_rebuild.stop(); info!("{}", measure_rebuild); @@ -890,7 +886,7 @@ pub fn bank_from_snapshot_archives( let mut measure_verify = Measure::start("verify"); if !bank.verify_snapshot_bank( test_hash_calculation, - accounts_db_skip_shrink, + accounts_db_skip_shrink || !full_snapshot_archive_info.is_remote(), Some(full_snapshot_archive_info.slot()), ) && limit_load_slot_count_from_snapshot.is_none() { @@ -1579,7 +1575,6 @@ fn rebuild_bank_from_snapshots( verify_index: bool, accounts_db_config: Option, accounts_update_notifier: Option, - accounts_db_skip_shrink: bool, ) -> Result { let (full_snapshot_version, full_snapshot_root_paths) = verify_unpacked_snapshots_dir_and_version( @@ -1628,7 +1623,6 @@ fn rebuild_bank_from_snapshots( verify_index, accounts_db_config, accounts_update_notifier, - accounts_db_skip_shrink, ), }?, )