vinishjail97 commented on code in PR #621:
URL: https://github.com/apache/incubator-xtable/pull/621#discussion_r1909577799


##########
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:
   Does `expiredSnapshots.apply()` do a a list for the metadata folder for this 
? I was trying to understand how expensive this could be for example if we 
retained 2 days of manifests assuming we have a commit every 5min. 



-- 
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

Reply via email to