szetszwo commented on code in PR #1569:
URL: https://github.com/apache/ratis/pull/1569#discussion_r3882545021
##########
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:
When snapshots are mixed (with and without md5), let's retain all snapshots
with md5 and the newer snapshots without md5.
```
+++
b/ratis-server/src/main/java/org/apache/ratis/statemachine/impl/SimpleStateMachineStorage.java
@@ -120,11 +120,13 @@ public class SimpleStateMachineStorage implements
StateMachineStorage {
final List<SingleFileSnapshotInfo> allSnapshotFiles =
getSingleFileSnapshotInfos(stateMachineDir.toPath());
allSnapshotFiles.sort(Comparator.comparing(SingleFileSnapshotInfo::getIndex).reversed());
int numSnapshotsWithMd5 = 0;
+ int lastSnapshotsWithMd5 = -1;
int deleteIdx = -1;
for (int i = 0; i < allSnapshotFiles.size(); i++) {
final SingleFileSnapshotInfo snapshot = allSnapshotFiles.get(i);
if (snapshot.hasMd5()) {
+ lastSnapshotsWithMd5 = i;
if (++numSnapshotsWithMd5 == numSnapshotsRetained) {
// We have found the last snapshot with an MD5 file that needs to
be retained
deleteIdx = i + 1;
@@ -135,6 +137,14 @@ public class SimpleStateMachineStorage implements
StateMachineStorage {
}
}
+ // Before RATIS-244, the newest numSnapshotsRetained snapshots (with or
without md5) are retained.
+ // For backward compatibility:
+ // - All snapshots without MD5 : retain the newest
numSnapshotsRetained snapshots.
+ // - Snapshots mixed (with and without md5): retain all snapshots with
md5 and the newer snapshots without md5.
+ if (deleteIdx < 0 && allSnapshotFiles.size() > numSnapshotsRetained) {
+ deleteIdx = Math.max(numSnapshotsRetained, lastSnapshotsWithMd5 + 1);
+ }
+
if (deleteIdx > 0) {
allSnapshotFiles.subList(deleteIdx, allSnapshotFiles.size())
.stream()
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
