test_shrink_candidate_slots uses write cache (solana-labs#29145)

runtime/src/accounts_db.rs

@@ -13752,7 +13752,8 @@ pub mod tests {
     fn test_shrink_candidate_slots() {
         solana_logger::setup();
 
-        let accounts = AccountsDb::new_single_for_tests();
+        let mut accounts = AccountsDb::new_single_for_tests();
+        accounts.caching_enabled = true;
 
         let pubkey_count = 30000;
         let pubkeys: Vec<_> = (0..pubkey_count)
@@ -13772,7 +13773,8 @@ pub mod tests {
             accounts.store_for_tests(current_slot, &[(pubkey, &account)]);
         }
         let shrink_slot = current_slot;
-        accounts.add_root(current_slot);
+        accounts.get_accounts_delta_hash(current_slot);
+        accounts.add_root_and_flush_write_cache(current_slot);
 
         current_slot += 1;
         let pubkey_count_after_shrink = 25000;
@@ -13781,16 +13783,19 @@ pub mod tests {
         for pubkey in updated_pubkeys {
             accounts.store_for_tests(current_slot, &[(pubkey, &account)]);
         }
-        accounts.add_root(current_slot);
+        accounts.get_accounts_delta_hash(current_slot);
+        accounts.add_root_and_flush_write_cache(current_slot);
         accounts.clean_accounts_for_tests();
 
         assert_eq!(
             pubkey_count,
             accounts.all_account_count_in_append_vec(shrink_slot)
         );
 
-        // Only, try to shrink stale slots, nothing happens because 90/100
+        // Only, try to shrink stale slots, nothing happens because shrink ratio
         // is not small enough to do a shrink
+        // Note this shrink ratio had to change because we are WAY over-allocating append vecs when we flush the write cache at the moment.
+        accounts.shrink_ratio = AccountShrinkThreshold::TotalSpace { shrink_ratio: 0.4 };
         accounts.shrink_candidate_slots();
         assert_eq!(
             pubkey_count,

runtime/src/rent_collector.rs

@@ -35,12 +35,14 @@ impl Default for RentCollector {
 /// when rent is collected for this account, this is the action to apply to the account
 #[derive(Debug)]
 enum RentResult {
-    /// maybe collect rent later, leave account alone
-    LeaveAloneNoRent,
+    /// this account will never have rent collected from it
+    Exempt,
+    /// maybe we collect rent later, but not now
+    NoRentCollectionNow,
     /// collect rent
     CollectRent {
         new_rent_epoch: Epoch,
-        rent_due: u64, // lamports
+        rent_due: u64, // lamports, could be 0
     },
 }
 
@@ -127,7 +129,7 @@ impl RentCollector {
         filler_account_suffix: Option<&Pubkey>,
     ) -> CollectedInfo {
         match self.calculate_rent_result(address, account, filler_account_suffix) {
-            RentResult::LeaveAloneNoRent => CollectedInfo::default(),
+            RentResult::Exempt | RentResult::NoRentCollectionNow => CollectedInfo::default(),
             RentResult::CollectRent {
                 new_rent_epoch,
                 rent_due,
@@ -159,37 +161,33 @@ impl RentCollector {
         account: &impl ReadableAccount,
         filler_account_suffix: Option<&Pubkey>,
     ) -> RentResult {
-        if self.can_skip_rent_collection(address, account, filler_account_suffix) {
-            return RentResult::LeaveAloneNoRent;
+        if account.rent_epoch() > self.epoch {
+            // potentially rent paying account
+            // Maybe collect rent later, leave account alone for now.
+            return RentResult::NoRentCollectionNow;
+        }
+        if !self.should_collect_rent(address, account)
+            || crate::accounts_db::AccountsDb::is_filler_account_helper(
+                address,
+                filler_account_suffix,
+            )
+        {
+            // easy to determine this account should not consider having rent collected from it
+            return RentResult::Exempt;
         }
         match self.get_rent_due(account) {
-            // Rent isn't collected for the next epoch.
-            // Make sure to check exempt status again later in current epoch.
-            RentDue::Exempt => RentResult::LeaveAloneNoRent,
-            // Maybe collect rent later, leave account alone.
-            RentDue::Paying(0) => RentResult::LeaveAloneNoRent,
+            // account will not have rent collected ever
+            RentDue::Exempt => RentResult::Exempt,
+            // potentially rent paying account
+            // Maybe collect rent later, leave account alone for now.
+            RentDue::Paying(0) => RentResult::NoRentCollectionNow,
             // Rent is collected for next epoch.
             RentDue::Paying(rent_due) => RentResult::CollectRent {
                 new_rent_epoch: self.epoch + 1,
                 rent_due,
             },
         }
     }
-
-    /// Performs easy checks to see if rent collection can be skipped
-    fn can_skip_rent_collection(
-        &self,
-        address: &Pubkey,
-        account: &impl ReadableAccount,
-        filler_account_suffix: Option<&Pubkey>,
-    ) -> bool {
-        !self.should_collect_rent(address, account)
-            || account.rent_epoch() > self.epoch
-            || crate::accounts_db::AccountsDb::is_filler_account_helper(
-                address,
-                filler_account_suffix,
-            )
-    }
 }
 
 /// Information computed during rent collection
@@ -244,6 +242,43 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_calculate_rent_result() {
+        let mut rent_collector = RentCollector::default();
+
+        let mut account = AccountSharedData::default();
+        assert!(matches!(
+            rent_collector.calculate_rent_result(&Pubkey::default(), &account, None),
+            RentResult::NoRentCollectionNow,
+        ));
+
+        account.set_executable(true);
+        assert!(matches!(
+            rent_collector.calculate_rent_result(&Pubkey::default(), &account, None),
+            RentResult::Exempt
+        ));
+
+        account.set_executable(false);
+        assert!(matches!(
+            rent_collector.calculate_rent_result(&incinerator::id(), &account, None),
+            RentResult::Exempt
+        ));
+
+        for (rent_epoch, rent_due_expected) in [(2, 2), (3, 5)] {
+            rent_collector.epoch = rent_epoch;
+            account.set_rent_epoch(1);
+            let new_rent_epoch_expected = rent_collector.epoch + 1;
+            assert!(
+                matches!(
+                    rent_collector.calculate_rent_result(&Pubkey::default(), &account, None),
+                    RentResult::CollectRent{ new_rent_epoch, rent_due} if new_rent_epoch == new_rent_epoch_expected && rent_due == rent_due_expected,
+                ),
+                "{:?}",
+                rent_collector.calculate_rent_result(&Pubkey::default(), &account, None)
+            );
+        }
+    }
+
     #[test]
     fn test_collect_from_account_created_and_existing() {
         let old_lamports = 1000;