Add a filter_by_type param for purge_old_bank_snapshots (#31191)

* Add a type_select param for purge_old_bank_snapshots

* Use flags to make the function calls more readable

* Remove the extra purge calls

* replace select_type with filter_by_type

* Add test

* Use matches

* Fix CI test on reference

* use match and call do_purge once

* Let bank_snapshots_dir be TempDir

* remove account_paths in the test

* replace bank with _bank

* Remove create_snapshot_dirs_for_tests, will take the lastest from master

* Fix merge errors

core/tests/snapshots.rs

@@ -473,7 +473,11 @@ fn test_concurrent_snapshot_packaging(
 
     // Purge all the outdated snapshots, including the ones needed to generate the package
     // currently sitting in the channel
-    snapshot_utils::purge_old_bank_snapshots(bank_snapshots_dir, MAX_BANK_SNAPSHOTS_TO_RETAIN);
+    snapshot_utils::purge_old_bank_snapshots(
+        bank_snapshots_dir,
+        MAX_BANK_SNAPSHOTS_TO_RETAIN,
+        None,
+    );
 
     let mut bank_snapshots = snapshot_utils::get_bank_snapshots_pre(bank_snapshots_dir);
     bank_snapshots.sort_unstable();

local-cluster/tests/local_cluster.rs

@@ -1102,7 +1102,11 @@ fn test_incremental_snapshot_download_with_crossing_full_snapshot_interval_at_st
     // To restart, it is not enough to remove the old bank snapshot directories under snapshot/.
     // The old hardlinks under <account_path>/snapshot/<slot> should also be removed.
     // The purge call covers all of them.
-    snapshot_utils::purge_old_bank_snapshots(validator_snapshot_test_config.bank_snapshots_dir, 0);
+    snapshot_utils::purge_old_bank_snapshots(
+        validator_snapshot_test_config.bank_snapshots_dir,
+        0,
+        None,
+    );
     cluster.restart_node(
         &validator_identity.pubkey(),
         validator_info,

runtime/src/accounts_background_service.rs

@@ -402,6 +402,7 @@ impl SnapshotRequestHandler {
         snapshot_utils::purge_old_bank_snapshots(
             &self.snapshot_config.bank_snapshots_dir,
             MAX_BANK_SNAPSHOTS_TO_RETAIN,
+            None,
         );
         purge_old_snapshots_time.stop();
         total_time.stop();

runtime/src/snapshot_utils.rs

@@ -2936,6 +2936,7 @@ pub fn verify_snapshot_archive<P, Q, R>(
 pub fn purge_old_bank_snapshots(
     bank_snapshots_dir: impl AsRef<Path>,
     num_bank_snapshots_to_retain: usize,
+    filter_by_type: Option<BankSnapshotType>,
 ) {
     let do_purge = |mut bank_snapshots: Vec<BankSnapshotInfo>| {
         bank_snapshots.sort_unstable();
@@ -2954,8 +2955,12 @@ pub fn purge_old_bank_snapshots(
             })
     };
 
-    do_purge(get_bank_snapshots_pre(&bank_snapshots_dir));
-    do_purge(get_bank_snapshots_post(&bank_snapshots_dir));
+    let bank_snapshots = match filter_by_type {
+        Some(BankSnapshotType::Pre) => get_bank_snapshots_pre(&bank_snapshots_dir),
+        Some(BankSnapshotType::Post) => get_bank_snapshots_post(&bank_snapshots_dir),
+        None => get_bank_snapshots(&bank_snapshots_dir),
+    };
+    do_purge(bank_snapshots);
 }
 
 /// Get the snapshot storages for this bank
@@ -5535,4 +5540,31 @@ mod tests {
             "Ensure rebuilding bank from the highest snapshot dir results in the highest bank",
         );
     }
+
+    #[test]
+    fn test_purge_old_bank_snapshots() {
+        solana_logger::setup();
+
+        let genesis_config = GenesisConfig::default();
+        let bank_snapshots_dir = tempfile::TempDir::new().unwrap();
+        let _bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 10, 5);
+        // Keep bank in this scope so that its account_paths tmp dirs are not released, and purge_old_bank_snapshots
+        // can clear the account hardlinks correctly.
+
+        assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 10);
+
+        purge_old_bank_snapshots(&bank_snapshots_dir, 3, Some(BankSnapshotType::Pre));
+        assert_eq!(get_bank_snapshots_pre(&bank_snapshots_dir).len(), 3);
+
+        purge_old_bank_snapshots(&bank_snapshots_dir, 2, Some(BankSnapshotType::Post));
+        assert_eq!(get_bank_snapshots_post(&bank_snapshots_dir).len(), 2);
+
+        assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 5);
+
+        purge_old_bank_snapshots(&bank_snapshots_dir, 2, None);
+        assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 2);
+
+        purge_old_bank_snapshots(&bank_snapshots_dir, 0, None);
+        assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 0);
+    }
 }