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


##########
core/src/main/java/org/apache/iceberg/SystemConfigs.java:
##########
@@ -53,6 +53,17 @@ private SystemConfigs() {}
           Math.max(2, Runtime.getRuntime().availableProcessors()),
           Integer::parseUnsignedInt);
 
+  /**
+   * The size of the queue in ParallelIterable. This queue limits the memory 
usage of manifest
+   * reader.
+   */
+  public static final ConfigEntry<Integer> ITERATOR_QUEUE_SIZE =
+      new ConfigEntry<>(
+          "iceberg.iterator.queue-size",

Review Comment:
   the config naming implies (at least to me) that this is being applied to all 
iterators being used in Iceberg, which isn't the case



##########
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:
   @Heltman can you please add a few more tests to `TestParallelIterable` that 
would exercise this exact condition you're describing?



##########
core/src/main/java/org/apache/iceberg/util/ParallelIterable.java:
##########
@@ -34,6 +35,8 @@
 import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
 
 public class ParallelIterable<T> extends CloseableGroup implements 
CloseableIterable<T> {
+  public static final int ITERATOR_QUEUE_SIZE = 
SystemConfigs.ITERATOR_QUEUE_SIZE.value();

Review Comment:
   any particular reason to make this public? This doesn't seem to be used 
anywhere outside of this class. Also you probably could just use 
`SystemConfigs.ITERATOR_QUEUE_SIZE.value()` directly in L59



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