From 2fd9a4f373ac0a15b347bebeb802aa50ab404d51 Mon Sep 17 00:00:00 2001 From: Brooks Prumo Date: Wed, 17 Aug 2022 18:45:59 -0400 Subject: [PATCH] Add clean_accounts_for_tests() (#27200) --- accounts-bench/src/main.rs | 2 +- runtime/benches/accounts.rs | 2 +- runtime/src/accounts.rs | 2 +- runtime/src/accounts_db.rs | 101 +++++++++++++++++++----------------- runtime/tests/accounts.rs | 2 +- 5 files changed, 57 insertions(+), 52 deletions(-) diff --git a/accounts-bench/src/main.rs b/accounts-bench/src/main.rs index 987915d8c..3d1c18633 100644 --- a/accounts-bench/src/main.rs +++ b/accounts-bench/src/main.rs @@ -110,7 +110,7 @@ fn main() { for x in 0..iterations { if clean { let mut time = Measure::start("clean"); - accounts.accounts_db.clean_accounts(None, false, None); + accounts.accounts_db.clean_accounts_for_tests(); time.stop(); println!("{}", time); for slot in 0..num_slots { diff --git a/runtime/benches/accounts.rs b/runtime/benches/accounts.rs index ec4eea2fe..7160c2efa 100644 --- a/runtime/benches/accounts.rs +++ b/runtime/benches/accounts.rs @@ -178,7 +178,7 @@ fn bench_delete_dependencies(bencher: &mut Bencher) { accounts.add_root(i); } bencher.iter(|| { - accounts.accounts_db.clean_accounts(None, false, None); + accounts.accounts_db.clean_accounts_for_tests(); }); } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 9c7838938..86d14aaf7 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -3081,7 +3081,7 @@ mod tests { } } info!("done..cleaning.."); - accounts.accounts_db.clean_accounts(None, false, None); + accounts.accounts_db.clean_accounts_for_tests(); } fn load_accounts_no_store( diff --git a/runtime/src/accounts_db.rs b/runtime/src/accounts_db.rs index 4f2fa0a5b..caa4cc77f 100644 --- a/runtime/src/accounts_db.rs +++ b/runtime/src/accounts_db.rs @@ -2507,6 +2507,11 @@ impl AccountsDb { pubkeys } + /// Call clean_accounts() with the common parameters that tests/benches use. + pub fn clean_accounts_for_tests(&self) { + self.clean_accounts(None, false, None) + } + // Purge zero lamport accounts and older rooted account states as garbage // collection // Only remove those accounts where the entire rooted history of the account @@ -10000,7 +10005,7 @@ pub mod tests { // overwrite old rooted account version; only the r_slot_0_stores.count() should be // decremented db.store_uncached(2, &[(&pubkeys[0], &account)]); - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); { let slot_0_stores = &db.storage.get_slot_stores(0).unwrap(); let slot_1_stores = &db.storage.get_slot_stores(1).unwrap(); @@ -10439,7 +10444,7 @@ pub mod tests { //slot is gone accounts.print_accounts_stats("pre-clean"); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert!(accounts.storage.map.get(&0).is_none()); //new value is there @@ -10522,7 +10527,7 @@ pub mod tests { // Slot 1 should be removed, slot 0 cannot be removed because it still has // the latest update for pubkey 2 - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert!(accounts.storage.get_slot_stores(0).is_some()); assert!(accounts.storage.get_slot_stores(1).is_none()); @@ -10557,7 +10562,7 @@ pub mod tests { assert_eq!(accounts.accounts_index.ref_count_from_storage(&pubkey1), 3); assert_eq!(accounts.accounts_index.ref_count_from_storage(&pubkey2), 1); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); // Slots 0 and 1 should each have been cleaned because all of their // accounts are zero lamports assert!(accounts.storage.get_slot_stores(0).is_none()); @@ -10571,7 +10576,7 @@ pub mod tests { assert_eq!(accounts.accounts_index.ref_count_from_storage(&pubkey1), 1); assert_eq!(accounts.accounts_index.ref_count_from_storage(&pubkey2), 0); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); // Slot 2 will now be cleaned, which will leave account 1 with a ref count of 0 assert!(accounts.storage.get_slot_stores(2).is_none()); assert_eq!(accounts.accounts_index.ref_count_from_storage(&pubkey1), 0); @@ -10598,7 +10603,7 @@ pub mod tests { // Slot 0 should be removed, and // zero-lamport account should be cleaned - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert!(accounts.storage.get_slot_stores(0).is_none()); assert!(accounts.storage.get_slot_stores(1).is_none()); @@ -10641,7 +10646,7 @@ pub mod tests { assert_eq!(accounts.alive_account_count_in_slot(0), 1); assert_eq!(accounts.alive_account_count_in_slot(1), 1); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); //now old state is cleaned up assert_eq!(accounts.alive_account_count_in_slot(0), 0); @@ -10675,7 +10680,7 @@ pub mod tests { accounts.print_accounts_stats(""); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); //Old state behind zero-lamport account is cleaned up assert_eq!(accounts.alive_account_count_in_slot(0), 0); @@ -10792,7 +10797,7 @@ pub mod tests { accounts.account_indexes.keys = None; } - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); //both zero lamport and normal accounts are cleaned up assert_eq!(accounts.alive_account_count_in_slot(0), 0); @@ -10883,7 +10888,7 @@ pub mod tests { assert_eq!(accounts.accounts_index.uncleaned_roots_len(), 1); //now uncleaned roots are cleaned up - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert_eq!(accounts.accounts_index.uncleaned_roots_len(), 0); } @@ -10900,7 +10905,7 @@ pub mod tests { assert_eq!(accounts.accounts_index.uncleaned_roots_len(), 1); //now uncleaned roots are cleaned up - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert_eq!(accounts.accounts_index.uncleaned_roots_len(), 0); } @@ -10912,7 +10917,7 @@ pub mod tests { // Create 100 accounts in slot 0 create_account(&accounts, &mut pubkeys, 0, 100, 0, 0); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); check_accounts(&accounts, &pubkeys, 0, 100, 1); // do some updates to those accounts and re-check @@ -10948,7 +10953,7 @@ pub mod tests { // Modify first 20 of the accounts from slot 0 in slot 2 modify_accounts(&accounts, &pubkeys, latest_slot, 20, 4); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); // Overwrite account 31 from slot 0 with lamports=0 into slot 2. // Slot 2 should now have 20 + 1 = 21 accounts let account = AccountSharedData::new(0, 0, AccountSharedData::default().owner()); @@ -10962,7 +10967,7 @@ pub mod tests { accounts.add_root(latest_slot); assert!(check_storage(&accounts, 2, 31)); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); // The first 20 accounts of slot 0 have been updated in slot 2, as well as // accounts 30 and 31 (overwritten with zero-lamport accounts in slot 1 and // slot 2 respectively), so only 78 accounts are left in slot 0's storage entries. @@ -11102,7 +11107,7 @@ pub mod tests { accounts.print_accounts_stats("pre_purge"); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); accounts.print_accounts_stats("post_purge"); @@ -11167,7 +11172,7 @@ pub mod tests { info!("ancestors: {:?}", ancestors); let hash = accounts.update_accounts_hash_test(current_slot, &ancestors); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert_eq!( accounts.update_accounts_hash_test(current_slot, &ancestors), @@ -11234,7 +11239,7 @@ pub mod tests { accounts.print_accounts_stats("accounts"); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); accounts.print_accounts_stats("accounts_post_purge"); let accounts = reconstruct_accounts_db_via_serialization(&accounts, current_slot); @@ -11320,7 +11325,7 @@ pub mod tests { fn test_accounts_purge_chained_purge_before_snapshot_restore() { solana_logger::setup(); with_chained_zero_lamport_accounts(|accounts, current_slot| { - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); reconstruct_accounts_db_via_serialization(&accounts, current_slot) }); } @@ -11331,7 +11336,7 @@ pub mod tests { with_chained_zero_lamport_accounts(|accounts, current_slot| { let accounts = reconstruct_accounts_db_via_serialization(&accounts, current_slot); accounts.print_accounts_stats("after_reconstruct"); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); reconstruct_accounts_db_via_serialization(&accounts, current_slot) }); } @@ -12095,7 +12100,7 @@ pub mod tests { accounts.print_count_and_status("before reconstruct"); let accounts = reconstruct_accounts_db_via_serialization(&accounts, current_slot); accounts.print_count_and_status("before purge zero"); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); accounts.print_count_and_status("after purge zero"); assert_load_account(&accounts, current_slot, pubkey, old_lamport); @@ -12156,7 +12161,7 @@ pub mod tests { accounts.print_accounts_stats("Post-B pre-clean"); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); info!("post B"); accounts.print_accounts_stats("Post-B"); @@ -12196,7 +12201,7 @@ pub mod tests { accounts.get_accounts_delta_hash(current_slot); accounts.add_root(current_slot); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); accounts.print_accounts_stats("Post-D clean"); @@ -12286,7 +12291,7 @@ pub mod tests { current_slot += 1; assert_eq!(3, accounts.ref_count_for_pubkey(&pubkey1)); accounts.store_uncached(current_slot, &[(&pubkey1, &zero_lamport_account)]); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert_eq!( // Removed one reference from the dead slot (reference only counted once @@ -12311,9 +12316,9 @@ pub mod tests { // If step C and step D should be purged, snapshot restore would cause // pubkey1 to be revived as the state of step A. // So, prevent that from happening by introducing refcount - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); let accounts = reconstruct_accounts_db_via_serialization(&accounts, current_slot); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); info!("pubkey: {}", pubkey1); accounts.print_accounts_stats("pre_clean"); @@ -12328,10 +12333,10 @@ pub mod tests { accounts.add_root(current_slot); // Do clean - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); // 2nd clean needed to clean-up pubkey1 - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); // Ensure pubkey2 is cleaned from the index finally assert_not_load_account(&accounts, current_slot, pubkey1); @@ -12472,7 +12477,7 @@ pub mod tests { accounts.get_accounts_delta_hash(current_slot); accounts.add_root(current_slot); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert_eq!( pubkey_count, @@ -12561,7 +12566,7 @@ pub mod tests { } accounts.get_accounts_delta_hash(current_slot); accounts.add_root(current_slot); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert_eq!( pubkey_count, @@ -12846,7 +12851,7 @@ pub mod tests { accounts.get_accounts_delta_hash(current_slot); accounts.add_root(current_slot); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert_eq!( pubkey_count, @@ -13056,7 +13061,7 @@ pub mod tests { accounts.flush_accounts_cache(true, None); // clear out the dirty keys - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); // flush 1 accounts.get_accounts_delta_hash(1); @@ -13068,11 +13073,11 @@ pub mod tests { // clean to remove pubkey1 from 0, // shrink to shrink pubkey1 from 0 // then another clean to remove pubkey1 from slot 1 - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); accounts.shrink_candidate_slots(); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); accounts.print_accounts_stats("post-clean"); assert_eq!(accounts.accounts_index.ref_count_from_storage(&pubkey1), 0); @@ -13100,12 +13105,12 @@ pub mod tests { accounts.store_uncached(1, &[(key, &account)]); } accounts.add_root(1); - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); accounts.shrink_all_slots(false, None); // Clean again to flush the dirty stores // and allow them to be recycled in the next step - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); accounts.print_accounts_stats("post-shrink"); let num_stores = accounts.recycle_stores.read().unwrap().entry_count(); assert!(num_stores > 0); @@ -13425,9 +13430,9 @@ pub mod tests { db.add_root(0); db.add_root(1); - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); db.flush_accounts_cache(true, None); - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); db.add_root(2); assert_eq!(db.read_only_accounts_cache.cache_len(), 0); @@ -13475,7 +13480,7 @@ pub mod tests { db.add_root(1); // Clean should not remove anything yet as nothing has been flushed - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); let account = db .do_load( &Ancestors::default(), @@ -13491,7 +13496,7 @@ pub mod tests { // Flush, then clean again. Should not need another root to initiate the cleaning // because `accounts_index.uncleaned_roots` should be correct db.flush_accounts_cache(true, None); - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); assert!(db .do_load( &Ancestors::default(), @@ -13556,7 +13561,7 @@ pub mod tests { // Flush, then clean. Should not need another root to initiate the cleaning // because `accounts_index.uncleaned_roots` should be correct db.flush_accounts_cache(true, None); - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); // The `zero_lamport_account_key` is still alive in slot 1, so refcount for the // pubkey should be 2 @@ -13716,7 +13721,7 @@ pub mod tests { // Run clean, unrooted slot 1 should not be purged, and still readable from the cache, // because we're still doing a scan on it. - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); let account = db .do_load( &scan_ancestors, @@ -13730,7 +13735,7 @@ pub mod tests { // When the scan is over, clean should not panic and should not purge something // still in the cache. scan_tracker.exit().unwrap(); - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); let account = db .do_load( &scan_ancestors, @@ -14332,7 +14337,7 @@ pub mod tests { // Checking that the uncleaned_pubkeys are not pre-maturely removed // such that when the slots are rooted, and can actually be cleaned, then the // delta keys are still there. - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); db.print_accounts_stats("post-clean1"); // Check stores > 0 @@ -14347,12 +14352,12 @@ pub mod tests { db.store_uncached(2, &[(&account_key1, &account3)]); db.get_accounts_delta_hash(2); - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); db.print_accounts_stats("post-clean2"); // root slots 1 db.add_root(1); - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); db.print_accounts_stats("post-clean3"); @@ -14361,7 +14366,7 @@ pub mod tests { db.add_root(3); // Check that we can clean where max_root=3 and slot=2 is not rooted - db.clean_accounts(None, false, None); + db.clean_accounts_for_tests(); assert!(db.uncleaned_pubkeys.is_empty()); @@ -15176,7 +15181,7 @@ pub mod tests { // The later rooted zero-lamport update to `shared_key` cannot be cleaned // because it is kept alive by the unrooted slot. - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert!(accounts .accounts_index .get_account_read_entry(&shared_key) @@ -15186,7 +15191,7 @@ pub mod tests { accounts.purge_slot(slot0, 0, true); // Now clean should clean up the remaining key - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); assert!(accounts .accounts_index .get_account_read_entry(&shared_key) diff --git a/runtime/tests/accounts.rs b/runtime/tests/accounts.rs index a055d62da..d272e738a 100644 --- a/runtime/tests/accounts.rs +++ b/runtime/tests/accounts.rs @@ -65,7 +65,7 @@ fn test_shrink_and_clean() { // let's dance. for _ in 0..10 { - accounts.clean_accounts(None, false, None); + accounts.clean_accounts_for_tests(); std::thread::sleep(std::time::Duration::from_millis(100)); }