Re: [PR] [lake/iceberg] Support snapshot expiration for lake tables [fluss]

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

vamossagar12 commented on code in PR #2881:
URL: https://github.com/apache/fluss/pull/2881#discussion_r3143190578


##########
fluss-lake/fluss-lake-iceberg/src/main/java/org/apache/fluss/lake/iceberg/tiering/IcebergLakeCommitter.java:
##########
@@ -262,6 +279,22 @@ public void close() throws Exception {
         }
     }
 
+    /**
+     * Expires old snapshots from the Iceberg table. Uses Iceberg's built-in snapshot expiration
+     * which respects table properties like {@code history.expire.max-snapshot-age-ms} and {@code
+     * history.expire.min-snapshots-to-keep}.
+     */
+    private void expireSnapshots() {
+        try {
+            ExpireSnapshots expireSnapshots =
+                    icebergTable.expireSnapshots().cleanExpiredFiles(true);

Review Comment:
   You're right — with aggressive expiration settings (e.g., `min-snapshots-to-keep`=1 and `max-snapshot-age-ms`=1ms), the expired snapshot could cause `getMissingLakeSnapshot()` to fail during recovery because `icebergTable.snapshot(expiredId)` returns null.
   
   I've fixed this by making `getMissingLakeSnapshot()` handle expired snapshots gracefully, similar to how Paimon's `SnapshotManager.pickOrLatest()` works. Instead of throwing IllegalStateException, the code now logs a warning and falls through to return the latest Fluss-committed live snapshot for recovery. Since Iceberg never expires the current snapshot, the latest Fluss-committed snapshot is guaranteed to be newer than the expired one.
   
   Added test `testSnapshotExpirationDoesNotBreakRecovery` to validate this scenario. Let me know what you think!
   
   



-- 
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@fluss.apache.org

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