signal to upsert whether reclaims are important (#18900)

* signal to upsert whether reclaims are important

* reclaims_must_be_empty -> previous_slot_entry_was_cached

* UPSERT_RECLAIMS_MUST_BE_EMPTY_FALSE -> UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE

runtime/benches/accounts_index.rs

@@ -29,6 +29,7 @@ fn bench_accounts_index(bencher: &mut Bencher) {
                 &AccountSecondaryIndexes::default(),
                 AccountInfo::default(),
                 &mut reclaims,
+                false,
             );
         }
     }
@@ -46,6 +47,7 @@ fn bench_accounts_index(bencher: &mut Bencher) {
                 &AccountSecondaryIndexes::default(),
                 AccountInfo::default(),
                 &mut reclaims,
+                false,
             );
             reclaims.clear();
         }

runtime/src/accounts_db.rs

@@ -5153,11 +5153,14 @@ impl AccountsDb {
         ret
     }
 
+    // previous_slot_entry_was_cached = true means we just need to assert that after this update is complete
+    //  that there are no items we would have put in reclaims that are not cached
     fn update_index(
         &self,
         slot: Slot,
         infos: Vec<AccountInfo>,
         accounts: &[(&Pubkey, &impl ReadableAccount)],
+        previous_slot_entry_was_cached: bool,
     ) -> SlotList<AccountInfo> {
         let mut reclaims = SlotList::<AccountInfo>::with_capacity(infos.len() * 2);
         for (info, pubkey_account) in infos.into_iter().zip(accounts.iter()) {
@@ -5170,6 +5173,7 @@ impl AccountsDb {
                 &self.account_indexes,
                 info,
                 &mut reclaims,
+                previous_slot_entry_was_cached,
             );
         }
         reclaims
@@ -5711,11 +5715,13 @@ impl AccountsDb {
             .fetch_add(store_accounts_time.as_us(), Ordering::Relaxed);
         let mut update_index_time = Measure::start("update_index");
 
+        let previous_slot_entry_was_cached = self.caching_enabled && is_cached_store;
+
         // If the cache was flushed, then because `update_index` occurs
         // after the account are stored by the above `store_accounts_to`
         // call and all the accounts are stored, all reads after this point
         // will know to not check the cache anymore
-        let mut reclaims = self.update_index(slot, infos, accounts);
+        let mut reclaims = self.update_index(slot, infos, accounts, previous_slot_entry_was_cached);
 
         // For each updated account, `reclaims` should only have at most one
         // item (if the account was previously updated in this slot).
@@ -5723,7 +5729,7 @@ impl AccountsDb {
         // to anything that needs to be cleaned in the backing storage
         // entries
         if self.caching_enabled {
-            reclaims.retain(|(_, r)| r.store_id != CACHE_VIRTUAL_STORAGE_ID);
+            reclaims.retain(|(_, r)| !r.is_cached());
 
             if is_cached_store {
                 assert!(reclaims.is_empty());
@@ -9848,6 +9854,8 @@ pub mod tests {
         );
     }
 
+    const UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE: bool = false;
+
     #[test]
     fn test_delete_dependencies() {
         solana_logger::setup();
@@ -9888,6 +9896,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             info0,
             &mut reclaims,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         accounts_index.upsert(
             1,
@@ -9897,6 +9906,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             info1.clone(),
             &mut reclaims,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         accounts_index.upsert(
             1,
@@ -9906,6 +9916,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             info1,
             &mut reclaims,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         accounts_index.upsert(
             2,
@@ -9915,6 +9926,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             info2.clone(),
             &mut reclaims,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         accounts_index.upsert(
             2,
@@ -9924,6 +9936,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             info2,
             &mut reclaims,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         accounts_index.upsert(
             3,
@@ -9933,6 +9946,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             info3,
             &mut reclaims,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         accounts_index.add_root(0, false);
         accounts_index.add_root(1, false);

runtime/src/accounts_index.rs

@@ -203,11 +203,17 @@ impl<T: IsCached> WriteAccountMapEntry<T> {
         pubkey: &Pubkey,
         new_value: AccountMapEntry<T>,
         reclaims: &mut SlotList<T>,
+        previous_slot_entry_was_cached: bool,
     ) {
         match w_account_maps.entry(*pubkey) {
             Entry::Occupied(mut occupied) => {
                 let current = occupied.get_mut();
-                Self::lock_and_update_slot_list(current, &new_value, reclaims);
+                Self::lock_and_update_slot_list(
+                    current,
+                    &new_value,
+                    reclaims,
+                    previous_slot_entry_was_cached,
+                );
             }
             Entry::Vacant(vacant) => {
                 vacant.insert(new_value);
@@ -222,9 +228,15 @@ impl<T: IsCached> WriteAccountMapEntry<T> {
         pubkey: &Pubkey,
         new_value: &AccountMapEntry<T>,
         reclaims: &mut SlotList<T>,
+        previous_slot_entry_was_cached: bool,
     ) -> bool {
         if let Some(current) = r_account_maps.get(pubkey) {
-            Self::lock_and_update_slot_list(current, new_value, reclaims);
+            Self::lock_and_update_slot_list(
+                current,
+                new_value,
+                reclaims,
+                previous_slot_entry_was_cached,
+            );
             true
         } else {
             false
@@ -235,10 +247,17 @@ impl<T: IsCached> WriteAccountMapEntry<T> {
         current: &Arc<AccountMapEntryInner<T>>,
         new_value: &AccountMapEntry<T>,
         reclaims: &mut SlotList<T>,
+        previous_slot_entry_was_cached: bool,
     ) {
         let mut slot_list = current.slot_list.write().unwrap();
         let (slot, new_entry) = new_value.slot_list.write().unwrap().remove(0);
-        let addref = Self::update_slot_list(&mut slot_list, slot, new_entry, reclaims);
+        let addref = Self::update_slot_list(
+            &mut slot_list,
+            slot,
+            new_entry,
+            reclaims,
+            previous_slot_entry_was_cached,
+        );
         if addref {
             Self::addref(&current.ref_count);
         }
@@ -251,6 +270,7 @@ impl<T: IsCached> WriteAccountMapEntry<T> {
         slot: Slot,
         account_info: T,
         reclaims: &mut SlotList<T>,
+        previous_slot_entry_was_cached: bool,
     ) -> bool {
         let mut addref = !account_info.is_cached();
 
@@ -258,11 +278,16 @@ impl<T: IsCached> WriteAccountMapEntry<T> {
         for list_index in 0..list.len() {
             let (s, previous_update_value) = &list[list_index];
             if *s == slot {
-                addref = addref && previous_update_value.is_cached();
+                let previous_was_cached = previous_update_value.is_cached();
+                addref = addref && previous_was_cached;
 
                 let mut new_item = (slot, account_info);
                 std::mem::swap(&mut new_item, &mut list[list_index]);
-                reclaims.push(new_item);
+                if previous_slot_entry_was_cached {
+                    assert!(previous_was_cached);
+                } else {
+                    reclaims.push(new_item);
+                }
                 list[(list_index + 1)..]
                     .iter()
                     .for_each(|item| assert!(item.0 != slot));
@@ -281,7 +306,7 @@ impl<T: IsCached> WriteAccountMapEntry<T> {
     pub fn update(&mut self, slot: Slot, account_info: T, reclaims: &mut SlotList<T>) {
         let mut addref = !account_info.is_cached();
         self.slot_list_mut(|list| {
-            addref = Self::update_slot_list(list, slot, account_info, reclaims);
+            addref = Self::update_slot_list(list, slot, account_info, reclaims, false);
         });
         if addref {
             // If it's the first non-cache insert, also bump the stored ref count
@@ -1520,6 +1545,7 @@ impl<T: IsCached> AccountsIndex<T> {
         account_indexes: &AccountSecondaryIndexes,
         account_info: T,
         reclaims: &mut SlotList<T>,
+        previous_slot_entry_was_cached: bool,
     ) {
         // We don't atomically update both primary index and secondary index together.
         // This certainly creates a small time window with inconsistent state across the two indexes.
@@ -1536,10 +1562,21 @@ impl<T: IsCached> AccountsIndex<T> {
         let map = &self.account_maps[self.bin_calculator.bin_from_pubkey(pubkey)];
 
         let r_account_maps = map.read().unwrap();
-        if !WriteAccountMapEntry::update_key_if_exists(r_account_maps, pubkey, &new_item, reclaims)
+        if !WriteAccountMapEntry::update_key_if_exists(
-        {
+            r_account_maps,
+            pubkey,
+            &new_item,
+            reclaims,
+            previous_slot_entry_was_cached,
+        ) {
             let w_account_maps = map.write().unwrap();
-            WriteAccountMapEntry::upsert(w_account_maps, pubkey, new_item, reclaims);
+            WriteAccountMapEntry::upsert(
+                w_account_maps,
+                pubkey,
+                new_item,
+                reclaims,
+                previous_slot_entry_was_cached,
+            );
         }
         self.update_secondary_indexes(pubkey, account_owner, account_data, account_indexes);
     }
@@ -2586,6 +2623,8 @@ pub mod tests {
         assert!(index.include_key(&pk2));
     }
 
+    const UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE: bool = false;
+
     #[test]
     fn test_insert_no_ancestors() {
         let key = Keypair::new();
@@ -2599,6 +2638,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(gc.is_empty());
 
@@ -2735,6 +2775,7 @@ pub mod tests {
                 &AccountSecondaryIndexes::default(),
                 account_infos[0].clone(),
                 &mut gc,
+                UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
             );
         } else {
             let items = vec![(key, account_infos[0].clone())];
@@ -2769,6 +2810,7 @@ pub mod tests {
                 &AccountSecondaryIndexes::default(),
                 account_infos[1].clone(),
                 &mut gc,
+                UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
             );
         } else {
             let items = vec![(key, account_infos[1].clone())];
@@ -2837,6 +2879,7 @@ pub mod tests {
             &key.pubkey(),
             &new_entry,
             &mut SlotList::default(),
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         ));
         assert_eq!(
             (slot, account_info),
@@ -2850,6 +2893,7 @@ pub mod tests {
             &key.pubkey(),
             new_entry,
             &mut SlotList::default(),
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert_eq!(1, account_maps_len_expensive(&index));
 
@@ -2879,6 +2923,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(gc.is_empty());
 
@@ -2903,6 +2948,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(gc.is_empty());
 
@@ -2936,6 +2982,7 @@ pub mod tests {
                 &AccountSecondaryIndexes::default(),
                 true,
                 &mut vec![],
+                UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
             );
             new_pubkey
         })
@@ -2952,6 +2999,7 @@ pub mod tests {
                 &AccountSecondaryIndexes::default(),
                 true,
                 &mut vec![],
+                UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
             );
         }
 
@@ -3083,6 +3131,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(iter.next().is_none());
     }
@@ -3108,6 +3157,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(gc.is_empty());
 
@@ -3222,6 +3272,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(gc.is_empty());
         let (list, idx) = index.get(&key.pubkey(), Some(&ancestors), None).unwrap();
@@ -3237,6 +3288,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             false,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert_eq!(gc, vec![(0, true)]);
         let (list, idx) = index.get(&key.pubkey(), Some(&ancestors), None).unwrap();
@@ -3258,6 +3310,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(gc.is_empty());
         index.upsert(
@@ -3268,6 +3321,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             false,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(gc.is_empty());
         let (list, idx) = index.get(&key.pubkey(), Some(&ancestors), None).unwrap();
@@ -3290,6 +3344,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(gc.is_empty());
         index.upsert(
@@ -3300,6 +3355,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             false,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         index.upsert(
             2,
@@ -3309,6 +3365,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         index.upsert(
             3,
@@ -3318,6 +3375,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         index.add_root(0, false);
         index.add_root(1, false);
@@ -3330,6 +3388,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             true,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
 
         // Updating index should not purge older roots, only purges
@@ -3373,6 +3432,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             12,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert_eq!(1, account_maps_len_expensive(&index));
 
@@ -3384,6 +3444,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             10,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert_eq!(1, account_maps_len_expensive(&index));
 
@@ -3403,6 +3464,7 @@ pub mod tests {
             &AccountSecondaryIndexes::default(),
             9,
             &mut gc,
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert_eq!(1, account_maps_len_expensive(&index));
     }
@@ -3478,6 +3540,7 @@ pub mod tests {
                 secondary_indexes,
                 true,
                 &mut vec![],
+                UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
             );
         }
 
@@ -3650,6 +3713,7 @@ pub mod tests {
             &secondary_indexes,
             true,
             &mut vec![],
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(secondary_index.index.is_empty());
         assert!(secondary_index.reverse_index.is_empty());
@@ -3663,6 +3727,7 @@ pub mod tests {
             &secondary_indexes,
             true,
             &mut vec![],
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert!(secondary_index.index.is_empty());
         assert!(secondary_index.reverse_index.is_empty());
@@ -3785,6 +3850,7 @@ pub mod tests {
             secondary_indexes,
             true,
             &mut vec![],
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
 
         // Now write a different mint index for the same account
@@ -3796,6 +3862,7 @@ pub mod tests {
             secondary_indexes,
             true,
             &mut vec![],
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
 
         // Both pubkeys will now be present in the index
@@ -3815,6 +3882,7 @@ pub mod tests {
             secondary_indexes,
             true,
             &mut vec![],
+            UPSERT_PREVIOUS_SLOT_ENTRY_WAS_CACHED_FALSE,
         );
         assert_eq!(secondary_index.get(&secondary_key1), vec![account_key]);