From 4fde0b0d4a22844fbb1fcc1f68a4023d1748d9ea Mon Sep 17 00:00:00 2001
From: "Jeff Washington (jwash)" <jeff.washington@solana.com>
Date: Fri, 9 Jun 2023 11:28:49 -0500
Subject: [PATCH] remove packed ancient append vec from shrink candidate list
 (#32022)

---
 runtime/src/ancient_append_vecs.rs | 149 ++++++++++++++++-------------
 1 file changed, 85 insertions(+), 64 deletions(-)

diff --git a/runtime/src/ancient_append_vecs.rs b/runtime/src/ancient_append_vecs.rs
index cedc124988..eb7e6da5b3 100644
--- a/runtime/src/ancient_append_vecs.rs
+++ b/runtime/src/ancient_append_vecs.rs
@@ -534,6 +534,9 @@ impl AccountsDb {
                 shrink_in_progress,
                 true,
             );
+
+            // If the slot is dead, remove the need to shrink the storage as the storage entries will be purged.
+            self.shrink_candidate_slots.lock().unwrap().remove(&slot);
         }
         self.handle_dropped_roots_for_ancient(dropped_roots.into_iter());
         metrics.accumulate(&write_ancient_accounts.metrics);
@@ -1342,75 +1345,93 @@ pub mod tests {
         // all accounts have 1 ref
         // nothing shrunk, so all storages and roots should be removed
         // or all slots shrunk so no roots or storages should be removed
-        for all_slots_shrunk in [false, true] {
-            for num_slots in 0..3 {
-                let (db, storages, slots, infos) = get_sample_storages(num_slots, None);
-                let accounts_per_storage = infos
-                    .iter()
-                    .zip(
-                        storages
-                            .iter()
-                            .map(|store| db.get_unique_accounts_from_storage(store)),
-                    )
-                    .collect::<Vec<_>>();
+        for in_shrink_candidate_slots in [false, true] {
+            for all_slots_shrunk in [false, true] {
+                for num_slots in 0..3 {
+                    let (db, storages, slots, infos) = get_sample_storages(num_slots, None);
+                    let accounts_per_storage = infos
+                        .iter()
+                        .zip(
+                            storages
+                                .iter()
+                                .map(|store| db.get_unique_accounts_from_storage(store)),
+                        )
+                        .collect::<Vec<_>>();
 
-                let accounts_to_combine = db.calc_accounts_to_combine(&accounts_per_storage);
-                let mut stats = ShrinkStatsSub::default();
-                let mut write_ancient_accounts = WriteAncientAccounts::default();
+                    let accounts_to_combine = db.calc_accounts_to_combine(&accounts_per_storage);
+                    let mut stats = ShrinkStatsSub::default();
+                    let mut write_ancient_accounts = WriteAncientAccounts::default();
 
-                slots.clone().for_each(|slot| {
-                    db.add_root(slot);
-                    assert!(db.storage.get_slot_storage_entry(slot).is_some());
-                });
-
-                let roots = db
-                    .accounts_index
-                    .roots_tracker
-                    .read()
-                    .unwrap()
-                    .alive_roots
-                    .get_all();
-                assert_eq!(roots, slots.clone().collect::<Vec<_>>());
-
-                if all_slots_shrunk {
-                    // make it look like each of the slots was shrunk
                     slots.clone().for_each(|slot| {
-                        write_ancient_accounts
-                            .shrinks_in_progress
-                            .insert(slot, db.get_store_for_shrink(slot, 1));
+                        db.add_root(slot);
+                        let storage = db.storage.get_slot_storage_entry(slot);
+                        assert!(storage.is_some());
+                        if in_shrink_candidate_slots {
+                            db.shrink_candidate_slots
+                                .lock()
+                                .unwrap()
+                                .insert(slot, storage.unwrap());
+                        }
+                    });
+
+                    let roots = db
+                        .accounts_index
+                        .roots_tracker
+                        .read()
+                        .unwrap()
+                        .alive_roots
+                        .get_all();
+                    assert_eq!(roots, slots.clone().collect::<Vec<_>>());
+
+                    if all_slots_shrunk {
+                        // make it look like each of the slots was shrunk
+                        slots.clone().for_each(|slot| {
+                            write_ancient_accounts
+                                .shrinks_in_progress
+                                .insert(slot, db.get_store_for_shrink(slot, 1));
+                        });
+                    }
+
+                    db.finish_combine_ancient_slots_packed_internal(
+                        accounts_to_combine,
+                        write_ancient_accounts,
+                        &mut stats,
+                    );
+
+                    slots.clone().for_each(|slot| {
+                        assert!(!db
+                            .shrink_candidate_slots
+                            .lock()
+                            .unwrap()
+                            .contains_key(&slot));
+                    });
+
+                    let roots_after = db
+                        .accounts_index
+                        .roots_tracker
+                        .read()
+                        .unwrap()
+                        .alive_roots
+                        .get_all();
+
+                    assert_eq!(
+                        roots_after,
+                        if all_slots_shrunk {
+                            slots.clone().collect::<Vec<_>>()
+                        } else {
+                            vec![]
+                        },
+                        "all_slots_shrunk: {all_slots_shrunk}"
+                    );
+                    slots.for_each(|slot| {
+                        let storage = db.storage.get_slot_storage_entry(slot);
+                        if all_slots_shrunk {
+                            assert!(storage.is_some());
+                        } else {
+                            assert!(storage.is_none());
+                        }
                     });
                 }
-
-                db.finish_combine_ancient_slots_packed_internal(
-                    accounts_to_combine,
-                    write_ancient_accounts,
-                    &mut stats,
-                );
-                let roots_after = db
-                    .accounts_index
-                    .roots_tracker
-                    .read()
-                    .unwrap()
-                    .alive_roots
-                    .get_all();
-
-                assert_eq!(
-                    roots_after,
-                    if all_slots_shrunk {
-                        slots.clone().collect::<Vec<_>>()
-                    } else {
-                        vec![]
-                    },
-                    "all_slots_shrunk: {all_slots_shrunk}"
-                );
-                slots.for_each(|slot| {
-                    let storage = db.storage.get_slot_storage_entry(slot);
-                    if all_slots_shrunk {
-                        assert!(storage.is_some());
-                    } else {
-                        assert!(storage.is_none());
-                    }
-                });
             }
         }
     }