clarify AccountsIndexScanResult::OnlyKeepInMemoryIfDirty (#31513)

runtime/src/accounts_db.rs

@@ -3301,7 +3301,7 @@ impl AccountsDb {
                                     useful += 1;
                                 }
                                 if useless {
-                                    AccountsIndexScanResult::None
+                                    AccountsIndexScanResult::OnlyKeepInMemoryIfDirty
                                 } else {
                                     AccountsIndexScanResult::KeepInMemory
                                 }
@@ -3798,7 +3798,7 @@ impl AccountsDb {
         self.accounts_index.scan(
             accounts.iter().map(|account| account.pubkey()),
             |pubkey, slots_refs, entry| {
-                let mut result = AccountsIndexScanResult::None;
+                let mut result = AccountsIndexScanResult::OnlyKeepInMemoryIfDirty;
                 if let Some((slot_list, ref_count)) = slots_refs {
                     let stored_account = &accounts[index];
                     let is_alive = slot_list.iter().any(|(slot, _acct_info)| {

runtime/src/accounts_index.rs

@@ -668,8 +668,8 @@ impl ScanSlotTracker {
 
 #[derive(Copy, Clone)]
 pub enum AccountsIndexScanResult {
-    /// if the entry is not in the in-memory index, do not add it, make no modifications to it
-    None,
+    /// if the entry is not in the in-memory index, do not add it unless the entry becomes dirty
+    OnlyKeepInMemoryIfDirty,
     /// keep the entry in the in-memory index
     KeepInMemory,
     /// reduce refcount by 1
@@ -1411,7 +1411,7 @@ impl<T: IndexValue, U: DiskIndexValue + From<T> + Into<T>> AccountsIndex<T, U> {
                                 true
                             }
                             AccountsIndexScanResult::KeepInMemory => true,
-                            AccountsIndexScanResult::None => false,
+                            AccountsIndexScanResult::OnlyKeepInMemoryIfDirty => false,
                         };
                     }
                     None => {

runtime/src/ancient_append_vecs.rs

@@ -347,7 +347,7 @@ impl AccountsDb {
                             if let Some(entry) = entry {
                                 entry.addref();
                             }
-                            AccountsIndexScanResult::None
+                            AccountsIndexScanResult::OnlyKeepInMemoryIfDirty
                         },
                         None,
                         true,
@@ -656,7 +656,7 @@ impl AccountsDb {
                         index -= 1;
                     }
                 }
-                AccountsIndexScanResult::None
+                AccountsIndexScanResult::OnlyKeepInMemoryIfDirty
             },
             None,
             false,
@@ -3093,7 +3093,7 @@ pub mod tests {
                 unrefed_pubkeys.iter(),
                 |k, slot_refs, _entry| {
                     assert_eq!(expected_ref_counts.remove(k).unwrap(), slot_refs.unwrap().1);
-                    AccountsIndexScanResult::None
+                    AccountsIndexScanResult::OnlyKeepInMemoryIfDirty
                 },
                 None,
                 false,

runtime/src/in_mem_accounts_index.rs

@@ -260,10 +260,14 @@ impl<T: IndexValue, U: DiskIndexValue + From<T> + Into<T>> InMemAccountsIndex<T,
         })
     }
 
+    /// lookup 'pubkey' in disk map.
+    /// If it is found, convert it to a cache entry and return the cache entry.
+    /// Cache entries from this function will always not be dirty.
     fn load_account_entry_from_disk(&self, pubkey: &Pubkey) -> Option<AccountMapEntry<T>> {
         let entry_disk = self.load_from_disk(pubkey)?; // returns None if not on disk
-
-        Some(self.disk_to_cache_entry(entry_disk.0, entry_disk.1))
+        let entry_cache = self.disk_to_cache_entry(entry_disk.0, entry_disk.1);
+        debug_assert!(!entry_cache.dirty());
+        Some(entry_cache)
     }
 
     /// lookup 'pubkey' by only looking in memory. Does not look on disk.
@@ -340,9 +344,12 @@ impl<T: IndexValue, U: DiskIndexValue + From<T> + Into<T>> InMemAccountsIndex<T,
                 match entry {
                     Entry::Occupied(occupied) => callback(Some(occupied.get())).1,
                     Entry::Vacant(vacant) => {
+                        debug_assert!(!disk_entry.dirty());
                         let (add_to_cache, rt) = callback(Some(&disk_entry));
-
-                        if add_to_cache {
+                        // We are holding a write lock to the in-memory map.
+                        // This pubkey is not in the in-memory map.
+                        // If the entry is now dirty, then it must be put in the cache or the modifications will be lost.
+                        if add_to_cache || disk_entry.dirty() {
                             stats.inc_mem_count(self.bin);
                             vacant.insert(disk_entry);
                         }