rdblue commented on code in PR #10691:
URL: https://github.com/apache/iceberg/pull/10691#discussion_r1683506642
##########
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:
> It's the iterable who is checked for being Closeable, so I would feel
obliged to call close on the instance that was given to me, for me to own it.
The purpose of this model is for cases where you use the iterable in a for
loop:
```java
try {
for (Item it : iterable) {
...
}
} finally {
// what should get closed here?
}
```
If you use that pattern, then there's nothing to close in the `finally`
block because the iterator is not available. We added closable iterables to
solve that. When you close an iterable, it closes all of the iterators that
were opened from it.
--
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
