nandorKollar commented on code in PR #4850:
URL: https://github.com/apache/polaris/pull/4850#discussion_r3458998584
##########
runtime/service/src/main/java/org/apache/polaris/service/task/BatchFileCleanupTaskHandler.java:
##########
@@ -77,17 +77,12 @@ public boolean handleTask(TaskEntity task, CallContext
callContext) {
missingFiles.size());
}
- // Schedule the deletion for each file asynchronously
- List<CompletableFuture<Void>> deleteFutures =
- validFiles.stream()
- .map(file -> super.tryDelete(tableId, authorizedFileIO, null,
file, null, 1))
- .toList();
+ CompletableFuture<Void> deleteFutures =
+ tryDelete(
+ tableId, authorizedFileIO, validFiles,
cleanupTask.type().getValue(), true, null, 1);
Review Comment:
We can do that, sure. However it appears that Iceberg doesn't let us
configure the thread pool, or at least I didn't find an option for it.
We should either copy the implementation of `deleteFiles` and use our own
executor, or check whether `fileIO instanceof SupportsBulkOperations` and call
`CatalogUtils.deleteFiles` only if that condition is met. In the latter case,
we wouldn't need to reimplement Iceberg's exception-logging logic; otherwise,
we could fall back to our own executor. Copying the entire method is probably
the better option.
--
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]
