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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
