peterxcli commented on code in PR #1569:
URL: https://github.com/apache/ratis/pull/1569#discussion_r3878487705
##########
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) {
Review Comment:
also there is a nit: we only need to do this if we havent found snapshot to
delete?
```suggestion
if (deleteIdx == -1 && allSnapshotFiles.size() > numSnapshotsRetained) {
```
##########
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) {
Review Comment:
I see `numSnapshotsRetained` is fetched from a policy, but looks like must
be the number of all snapshot files that has md5 then the logic here could be
valid.
--
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]
