[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #6217: Core: Fix deletion of old metadata files when METADATA_DELETE_AFTER_COMMIT_ENABLED is set

[GitBox <gi...@apache.org>]

amogh-jahagirdar commented on code in PR #6217:
URL: https://github.com/apache/iceberg/pull/6217#discussion_r1028798630


##########
core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java:
##########
@@ -414,16 +414,15 @@ private void deleteRemovedMetadataFiles(TableMetadata base, TableMetadata metada
             TableProperties.METADATA_DELETE_AFTER_COMMIT_ENABLED_DEFAULT);
 
     if (deleteAfterCommit) {
-      Set<TableMetadata.MetadataLogEntry> removedPreviousMetadataFiles =
-          Sets.newHashSet(base.previousFiles());
-      removedPreviousMetadataFiles.removeAll(metadata.previousFiles());
-      Tasks.foreach(removedPreviousMetadataFiles)
+      Set<TableMetadata.MetadataLogEntry> filesToDelete = Sets.newHashSet(base.previousFiles());
+      filesToDelete.addAll(metadata.previousFiles());
+      Tasks.foreach(filesToDelete)
           .noRetry()
           .suppressFailureWhenFinished()
           .onFailure(
-              (previousMetadataFile, exc) ->
+              (metadataFileToDelete, exc) ->
                   LOG.warn(
-                      "Delete failed for previous metadata file: {}", previousMetadataFile, exc))
+                      "Delete failed for previous metadata file: {}", metadataFileToDelete, exc))
           .run(previousMetadataFile -> io().deleteFile(previousMetadataFile.file()));

Review Comment:
   Should this use batch delete if applicable?  I don't think it's a blocker though, something we can get in a follow on if it's too intrusive.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org