Heltman commented on code in PR #7844:
URL: https://github.com/apache/iceberg/pull/7844#discussion_r1411654195


##########
core/src/main/java/org/apache/iceberg/util/ParallelIterable.java:
##########
@@ -67,10 +70,13 @@ private ParallelIterator(
                             try (Closeable ignored =
                                 (iterable instanceof Closeable) ? (Closeable) 
iterable : () -> {}) {
                               for (T item : iterable) {
-                                queue.add(item);
+                                queue.put(item);

Review Comment:
   @ConeyLiu I found `future.cancel` can't exit this loop, because 
`ConcurrentLinkedQueue` not check InterruptedException. So we need check 
`close` in this place, to avoid memory leak. 
   
   Actural, I found trino kill query, but this loop will continue add to queue 
until finished.
   
   We need use `LinkedBlockingQueue ` or add check iterator is `closed`, or 
both them.Just like below
   ```java
   for (T item : iterable) {
         if (closed) {
            queue.clear()
            return;
        }
         queue.put(item);
   }
   ```



-- 
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

Reply via email to