findepi commented on code in PR #10691:
URL: https://github.com/apache/iceberg/pull/10691#discussion_r1677634825
##########
core/src/main/java/org/apache/iceberg/util/ParallelIterable.java:
##########
@@ -88,16 +92,26 @@ private ParallelIterator(
@Override
public void close() {
// close first, avoid new task submit
- this.closed = true;
+ this.closed.set(true);
+
+ try (Closer closer = Closer.create()) {
+ yieldedTasks.forEach(closer::register);
+ yieldedTasks.clear();
- // cancel background tasks
- for (Future<?> taskFuture : taskFutures) {
- if (taskFuture != null && !taskFuture.isDone()) {
- taskFuture.cancel(true);
+ // TODO close input iterables that were not started yet
Review Comment:
the existing code (in main branch) closes iterables, not the iterators
derived from them
so to me it makes sense to reliably close all iterables.
i can back out this TODO though, if this is preferred.
--
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
