vamsikarnika commented on code in PR #621: URL: https://github.com/apache/incubator-xtable/pull/621#discussion_r1909831419
########## xtable-core/src/main/java/org/apache/xtable/iceberg/IcebergConversionTarget.java: ########## @@ -211,18 +222,34 @@ public void syncFilesForDiff(DataFilesDiff dataFilesDiff) { @Override public void completeSync() { - transaction - .expireSnapshots() - .expireOlderThan( - Instant.now().minus(snapshotRetentionInHours, ChronoUnit.HOURS).toEpochMilli()) - .deleteWith(this::safeDelete) // ensures that only metadata files are deleted - .cleanExpiredFiles(true) - .commit(); + boolean useInternalIcebergCleaner = useInternalCleaner(); + ExpireSnapshots expireSnapshots = + transaction + .expireSnapshots() + .expireOlderThan( + Instant.now().minus(snapshotRetentionInHours, ChronoUnit.HOURS).toEpochMilli()) + .cleanExpiredFiles(!useInternalIcebergCleaner); // is internal cleaner is enabled, disable iceberg cleaner + List<Snapshot> removedSnapshots = expireSnapshots.apply(); Review Comment: No, this will only operate on TableMetadata to compute the snapshots needs to be retained based on the provided retention time. Metadata file listing happens only as part of cleaning. -- 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: commits-unsubscr...@xtable.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org