Re: [PR] RATIS-2680. Maintain backward compatibility for cleanupOldSnapshots [ratis]

["amaliujia (via GitHub)" <gi...@apache.org>]

amaliujia commented on code in PR #1569:
URL: https://github.com/apache/ratis/pull/1569#discussion_r3891965756


##########
ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java:
##########
@@ -135,6 +135,14 @@ public void cleanupOldSnapshots(SnapshotRetentionPolicy snapshotRetentionPolicy)
       }
     }
 
+    // Backward compatibility: before MD5 files existed, all snapshots counted toward
+    // retention. When there are fewer MD5 snapshots than numSnapshotsRetained (e.g.
+    // all old snapshots without MD5, or old snapshots mixed with new ones after upgrade),
+    // fall back to retaining the newest numSnapshotsRetained snapshots regardless of MD5.
+    if (deleteIdx < 0 && allSnapshotFiles.size() > numSnapshotsRetained) {
+      deleteIdx = numSnapshotsRetained;
+    }
+

Review Comment:
   Ack. It makes sense to differentiate the corrupted snapshots after the upgrade with legit snapshots before the snapshots (both are lack of MD5 files).
   
   Have applied this code suggestion. Will ask @spacemonkd to review as well.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org