amogh-jahagirdar commented on a change in pull request #4052:
URL: https://github.com/apache/iceberg/pull/4052#discussion_r814070479
##########
File path: aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java
##########
@@ -100,6 +115,67 @@ public void deleteFile(String path) {
client().deleteObject(deleteRequest);
}
+ /**
+ * Deletes the given paths in a batched manner.
+ * <p>
+ * The paths are grouped by bucket, and deletion is triggered when we either
reach the configured batch size
+ * or have a final remainder batch for each bucket.
+ *
+ * @param paths paths to delete
+ */
+ @Override
+ public void deleteFiles(Iterable<String> paths) {
Review comment:
Actually, one issue with falling back to the normal delete
implementation in case the batch size is 1 is we currently don't have any
expectation on the exception being surfaced by FileIo#deleteFile in case of a
failure during deletion.
With deleteFiles we are keeping track of failures and then surfacing them at
the end (which I think is the right behavior for deleteFiles), so in the case
the batch size is 1 we would have to catch generic Exceptions and keep track of
failures that way which seems more messy. So now I'm thinking it makes more
sense to keep the generic batch removal logic (the multi-map state management
is light-weight). Let me know your thoughts @rdblue
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
