hash calculation adds really old slots to dirty_stores (backport #19434) (#19451)

* hash calculation adds really old slots to dirty_stores (#19434)

(cherry picked from commit 98bc694606)

* fix test compile error

Co-authored-by: Jeff Washington (jwash) <75863576+jeffwashington@users.noreply.github.com>
Co-authored-by: Jeff Washington (jwash) <wash678@gmail.com>

accounts-bench/src/main.rs

@@ -122,6 +122,7 @@ fn main() {
                 &ancestors,
                 None,
                 false,
+                None,
             );
             time_store.stop();
             if results != results_store {

runtime/src/accounts.rs

@@ -646,6 +646,7 @@ impl Accounts {
                 ancestors,
                 None,
                 can_cached_slot_be_unflushed,
+                None,
             )
             .1
     }

runtime/src/accounts_background_service.rs

@@ -107,7 +107,7 @@ impl SnapshotRequestHandler {
                 let previous_hash = if test_hash_calculation {
                     // We have to use the index version here.
                     // We cannot calculate the non-index way because cache has not been flushed and stores don't match reality.
-                    snapshot_root_bank.update_accounts_hash_with_index_option(true, false)
+                    snapshot_root_bank.update_accounts_hash_with_index_option(true, false, None)
                 } else {
                     Hash::default()
                 };
@@ -145,6 +145,7 @@ impl SnapshotRequestHandler {
                 let this_hash = snapshot_root_bank.update_accounts_hash_with_index_option(
                     use_index_hash_calculation,
                     test_hash_calculation,
+                    Some(snapshot_root_bank.epoch_schedule().slots_per_epoch),
                 );
                 let hash_for_testing = if test_hash_calculation {
                     assert_eq!(previous_hash, this_hash);

runtime/src/accounts_db.rs

@@ -4571,11 +4571,11 @@ impl AccountsDb {
     }
 
     pub fn update_accounts_hash(&self, slot: Slot, ancestors: &Ancestors) -> (Hash, u64) {
-        self.update_accounts_hash_with_index_option(true, false, slot, ancestors, None, false)
+        self.update_accounts_hash_with_index_option(true, false, slot, ancestors, None, false, None)
     }
 
     pub fn update_accounts_hash_test(&self, slot: Slot, ancestors: &Ancestors) -> (Hash, u64) {
-        self.update_accounts_hash_with_index_option(true, true, slot, ancestors, None, false)
+        self.update_accounts_hash_with_index_option(true, true, slot, ancestors, None, false, None)
     }
 
     fn scan_multiple_account_storages_one_slot<F, B>(
@@ -4695,6 +4695,25 @@ impl AccountsDb {
             .collect()
     }
 
+    // storages are sorted by slot and have range info.
+    // if we know slots_per_epoch, then add all stores older than slots_per_epoch to dirty_stores so clean visits these slots
+    fn mark_old_slots_as_dirty(&self, storages: &SortedStorages, slots_per_epoch: Option<Slot>) {
+        if let Some(slots_per_epoch) = slots_per_epoch {
+            let max = storages.range().end;
+            let acceptable_straggler_slot_count = 100; // do nothing special for these old stores which will likely get cleaned up shortly
+            let sub = slots_per_epoch + acceptable_straggler_slot_count;
+            let in_epoch_range_start = max.saturating_sub(sub);
+            for slot in storages.range().start..in_epoch_range_start {
+                if let Some(storages) = storages.get(slot) {
+                    storages.iter().for_each(|store| {
+                        self.dirty_stores
+                            .insert((slot, store.id.load(Ordering::Relaxed)), store.clone());
+                    });
+                }
+            }
+        }
+    }
+
     fn calculate_accounts_hash_helper(
         &self,
         use_index: bool,
@@ -4702,6 +4721,7 @@ impl AccountsDb {
         ancestors: &Ancestors,
         check_hash: bool,
         can_cached_slot_be_unflushed: bool,
+        slots_per_epoch: Option<Slot>,
     ) -> Result<(Hash, u64), BankHashVerificationError> {
         if !use_index {
             let accounts_cache_and_ancestors = if can_cached_slot_be_unflushed {
@@ -4721,6 +4741,8 @@ impl AccountsDb {
                 min_root,
                 Some(slot),
             );
+
+            self.mark_old_slots_as_dirty(&storages, slots_per_epoch);
             sort_time.stop();
 
             let timings = HashStats {
@@ -4750,6 +4772,7 @@ impl AccountsDb {
         expected_capitalization: Option<u64>,
         can_cached_slot_be_unflushed: bool,
         check_hash: bool,
+        slots_per_epoch: Option<Slot>,
     ) -> Result<(Hash, u64), BankHashVerificationError> {
         let (hash, total_lamports) = self.calculate_accounts_hash_helper(
             use_index,
@@ -4757,6 +4780,7 @@ impl AccountsDb {
             ancestors,
             check_hash,
             can_cached_slot_be_unflushed,
+            slots_per_epoch,
         )?;
         if debug_verify {
             // calculate the other way (store or non-store) and verify results match.
@@ -4766,6 +4790,7 @@ impl AccountsDb {
                 ancestors,
                 check_hash,
                 can_cached_slot_be_unflushed,
+                None,
             )?;
 
             let success = hash == hash_other
@@ -4784,6 +4809,7 @@ impl AccountsDb {
         ancestors: &Ancestors,
         expected_capitalization: Option<u64>,
         can_cached_slot_be_unflushed: bool,
+        slots_per_epoch: Option<Slot>,
     ) -> (Hash, u64) {
         let check_hash = false;
         let (hash, total_lamports) = self
@@ -4795,6 +4821,7 @@ impl AccountsDb {
                 expected_capitalization,
                 can_cached_slot_be_unflushed,
                 check_hash,
+                slots_per_epoch,
             )
             .unwrap(); // unwrap here will never fail since check_hash = false
         let mut bank_hashes = self.bank_hashes.write().unwrap();
@@ -5001,6 +5028,7 @@ impl AccountsDb {
                 None,
                 can_cached_slot_be_unflushed,
                 check_hash,
+                None,
             )?;
 
         if calculated_lamports != total_lamports {
@@ -8771,12 +8799,13 @@ pub mod tests {
         );
         db.add_root(some_slot);
         let check_hash = true;
-        assert!(db
+        for use_index in &[true, false] {
-            .calculate_accounts_hash_helper(false, some_slot, &ancestors, check_hash, false)
+            assert!(db
-            .is_err());
+                .calculate_accounts_hash_helper(
-        assert!(db
+                    *use_index, some_slot, &ancestors, check_hash, false, None
-            .calculate_accounts_hash_helper(true, some_slot, &ancestors, check_hash, false)
+                )
-            .is_err());
+                .is_err());
+        }
     }
 
     #[test]
@@ -8795,9 +8824,11 @@ pub mod tests {
         db.add_root(some_slot);
         let check_hash = true;
         assert_eq!(
-            db.calculate_accounts_hash_helper(false, some_slot, &ancestors, check_hash, false)
+            db.calculate_accounts_hash_helper(
-                .unwrap(),
+                false, some_slot, &ancestors, check_hash, false, None
-            db.calculate_accounts_hash_helper(true, some_slot, &ancestors, check_hash, false)
+            )
+            .unwrap(),
+            db.calculate_accounts_hash_helper(true, some_slot, &ancestors, check_hash, false, None)
                 .unwrap(),
         );
     }

runtime/src/bank.rs

@@ -4673,6 +4673,7 @@ impl Bank {
         &self,
         use_index: bool,
         debug_verify: bool,
+        slots_per_epoch: Option<Slot>,
     ) -> Hash {
         let (hash, total_lamports) = self
             .rc
@@ -4685,6 +4686,7 @@ impl Bank {
                 &self.ancestors,
                 Some(self.capitalization()),
                 false,
+                slots_per_epoch,
             );
         if total_lamports != self.capitalization() {
             datapoint_info!(
@@ -4705,7 +4707,7 @@ impl Bank {
     }
 
     pub fn update_accounts_hash(&self) -> Hash {
-        self.update_accounts_hash_with_index_option(true, false)
+        self.update_accounts_hash_with_index_option(true, false, None)
     }
 
     /// A snapshot bank should be purged of 0 lamport accounts which are not part of the hash