amogh-jahagirdar commented on code in PR #5459:
URL: https://github.com/apache/iceberg/pull/5459#discussion_r939747517
##########
core/src/main/java/org/apache/iceberg/CatalogUtil.java:
##########
@@ -151,20 +165,32 @@ private static void deleteFiles(FileIO io,
Set<ManifestFile> allManifests) {
.run(
manifest -> {
try (ManifestReader<?> reader = ManifestFiles.open(manifest,
io)) {
+ List<String> pathsToDelete = Lists.newArrayList();
for (ManifestEntry<?> entry : reader.entries()) {
// intern the file path because the weak key map uses
identity (==) instead of
// equals
String path = entry.file().path().toString().intern();
Boolean alreadyDeleted = deletedFiles.putIfAbsent(path,
true);
if (alreadyDeleted == null || !alreadyDeleted) {
try {
- io.deleteFile(path);
+ if (io instanceof SupportsBulkOperations) {
+ pathsToDelete.add(path);
+ } else {
+ io.deleteFile(path);
+ }
} catch (RuntimeException e) {
// this may happen if the map of deleted files gets
cleaned up by gc
LOG.warn("Delete failed for data file: {}", path, e);
}
}
}
+ if (io instanceof SupportsBulkOperations) {
+ try {
+ ((SupportsBulkOperations) io).deleteFiles(pathsToDelete);
+ } catch (Exception e) {
Review Comment:
Decided to be defensive against any exception so that we always having a
clear logging message on 191 that the bulk deletion failed. In current
S3FileIO#deleteFiles we are guaranteed to throw the bulk deletion failure, but
in case in future other exceptions get thrown or other file io implementations
throw other exceptions, we should always hit this case.
--
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]
