From a1112254a56fbe9561e6ecb8a44cc765dcd36556 Mon Sep 17 00:00:00 2001
From: Brooks Prumo <brooks@solana.com>
Date: Wed, 4 Aug 2021 16:42:42 -0500
Subject: [PATCH] Fix wrong old snapshot archives getting purged (#19061)

I introduced a bug where old snapshot archives were incorrectly purged.
Instead of purged to oldest, I was purged the newest...

The fix is to add a `reverse()` in the purge logic, and I've added a
test to catch this bug in the future.

Fixes #19057
---
 runtime/src/snapshot_utils.rs | 48 +++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs
index b87246e4fd..235d5399ad 100644
--- a/runtime/src/snapshot_utils.rs
+++ b/runtime/src/snapshot_utils.rs
@@ -1270,6 +1270,7 @@ where
     );
     let mut snapshot_archives = get_full_snapshot_archives(&snapshot_archives_dir);
     snapshot_archives.sort_unstable();
+    snapshot_archives.reverse();
     // Keep the oldest snapshot so we can always play the ledger from it.
     snapshot_archives.pop();
     let max_snaps = max(1, maximum_snapshots_to_retain);
@@ -2373,6 +2374,53 @@ mod tests {
         common_test_purge_old_snapshot_archives(&snapshot_names, 2, &expected_snapshots);
     }
 
+    /// Mimic a running node's behavior w.r.t. purging old snapshot archives.  Take snapshots in a
+    /// loop, and periodically purge old snapshot archives.  After purging, check to make sure the
+    /// snapshot archives on disk are correct.
+    #[test]
+    fn test_purge_old_full_snapshot_archives_in_the_loop() {
+        let snapshot_archives_dir = tempfile::TempDir::new().unwrap();
+        let maximum_snapshots_to_retain = 5;
+        let starting_slot: Slot = 42;
+
+        for slot in (starting_slot..).take(100) {
+            let full_snapshot_archive_file_name =
+                format!("snapshot-{}-{}.tar", slot, Hash::default());
+            let full_snapshot_archive_path = snapshot_archives_dir
+                .as_ref()
+                .join(full_snapshot_archive_file_name);
+            File::create(full_snapshot_archive_path).unwrap();
+
+            // don't purge-and-check until enough snapshot archives have been created
+            if slot < starting_slot + maximum_snapshots_to_retain as Slot {
+                continue;
+            }
+
+            // purge infrequently, so there will always be snapshot archives to purge
+            if slot % (maximum_snapshots_to_retain as Slot * 2) != 0 {
+                continue;
+            }
+
+            purge_old_snapshot_archives(&snapshot_archives_dir, maximum_snapshots_to_retain);
+            let mut full_snapshot_archives = get_full_snapshot_archives(&snapshot_archives_dir);
+            full_snapshot_archives.sort_unstable();
+            assert_eq!(
+                full_snapshot_archives.len(),
+                maximum_snapshots_to_retain + 1
+            );
+            assert_eq!(
+                *full_snapshot_archives.first().unwrap().slot(),
+                starting_slot
+            );
+            assert_eq!(*full_snapshot_archives.last().unwrap().slot(), slot);
+            for (i, full_snapshot_archive) in
+                full_snapshot_archives.iter().skip(1).rev().enumerate()
+            {
+                assert_eq!(*full_snapshot_archive.slot(), slot - i as Slot);
+            }
+        }
+    }
+
     #[test]
     fn test_purge_old_incremental_snapshot_archives() {
         let snapshot_archives_dir = tempfile::TempDir::new().unwrap();