Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2024-09-02 Thread via GitHub
github-actions[bot] commented on PR #7844: URL: https://github.com/apache/iceberg/pull/7844#issuecomment-2325403682 This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2024-07-12 Thread via GitHub
findepi commented on PR #7844: URL: https://github.com/apache/iceberg/pull/7844#issuecomment-2225627726 I created a PR aiming to make the queue bounded, but without requiring separate executor pool. The change is effectively transparent to class consumers. Please see https://github.com/apac

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2024-01-03 Thread via GitHub
Heltman commented on PR #7844: URL: https://github.com/apache/iceberg/pull/7844#issuecomment-1875283687 > I will add a some change for fix memory leak. And think about creating BlockingParallelIterable instead of change ParallelIterable. I add a new pr just fix memory leak. See #9402.

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-21 Thread via GitHub
findepi commented on PR #7844: URL: https://github.com/apache/iceberg/pull/7844#issuecomment-1866905125 > The problem is that planning uses a shared threadpool. Using a blocking queue would cause tasks to stall, which would then tie up the threads in the shared pool and cause all planni

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-19 Thread via GitHub
Heltman commented on PR #7844: URL: https://github.com/apache/iceberg/pull/7844#issuecomment-1862361851 I will add a some change for fix memory leak. And think about creating BlockingParallelIterable instead of change ParallelIterable. -- This is an automated message from the Apache Git

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-18 Thread via GitHub
lirui-apache commented on PR #7844: URL: https://github.com/apache/iceberg/pull/7844#issuecomment-1862080641 We have been using a blocking queue and shared thread pool for a while. We did hit some "dead lock" issue when running multi-stage queries with trino, because the iceberg split sourc

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-18 Thread via GitHub
Heltman commented on code in PR #7844: URL: https://github.com/apache/iceberg/pull/7844#discussion_r1430862160 ## core/src/main/java/org/apache/iceberg/util/ParallelIterable.java: ## @@ -34,6 +35,8 @@ import org.apache.iceberg.relocated.com.google.common.collect.Iterables; p

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-18 Thread via GitHub
Heltman commented on code in PR #7844: URL: https://github.com/apache/iceberg/pull/7844#discussion_r1430861244 ## core/src/main/java/org/apache/iceberg/util/ParallelIterable.java: ## @@ -67,10 +70,13 @@ private ParallelIterator( try (Closeable ignore

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-18 Thread via GitHub
Heltman commented on code in PR #7844: URL: https://github.com/apache/iceberg/pull/7844#discussion_r1430860065 ## core/src/main/java/org/apache/iceberg/util/ParallelIterable.java: ## @@ -53,7 +57,8 @@ public CloseableIterator iterator() { private final Iterator tasks;

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-18 Thread via GitHub
rdblue commented on code in PR #7844: URL: https://github.com/apache/iceberg/pull/7844#discussion_r1430498967 ## core/src/main/java/org/apache/iceberg/util/ParallelIterable.java: ## @@ -67,10 +70,13 @@ private ParallelIterator( try (Closeable ignored

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-18 Thread via GitHub
rdblue commented on code in PR #7844: URL: https://github.com/apache/iceberg/pull/7844#discussion_r1430498570 ## core/src/main/java/org/apache/iceberg/util/ParallelIterable.java: ## @@ -53,7 +57,8 @@ public CloseableIterator iterator() { private final Iterator tasks; p

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-18 Thread via GitHub
rdblue commented on code in PR #7844: URL: https://github.com/apache/iceberg/pull/7844#discussion_r1430496740 ## core/src/main/java/org/apache/iceberg/SystemConfigs.java: ## @@ -42,6 +42,17 @@ private SystemConfigs() {} Math.max(2, Runtime.getRuntime().availableProces

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-18 Thread via GitHub
rdblue commented on PR #7844: URL: https://github.com/apache/iceberg/pull/7844#issuecomment-1861228366 This class cannot use a blocking queue with the worker pool, so I'm -1 on this change. The problem is that planning uses a shared threadpool. Using a blocking queue would cause task

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-18 Thread via GitHub
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().availableProces

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-17 Thread via GitHub
Heltman commented on code in PR #7844: URL: https://github.com/apache/iceberg/pull/7844#discussion_r1429368849 ## core/src/main/java/org/apache/iceberg/util/ParallelIterable.java: ## @@ -67,10 +70,17 @@ private ParallelIterator( try (Closeable ignore

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-15 Thread via GitHub
danielcweeks commented on code in PR #7844: URL: https://github.com/apache/iceberg/pull/7844#discussion_r1428383394 ## core/src/main/java/org/apache/iceberg/util/ParallelIterable.java: ## @@ -67,10 +70,17 @@ private ParallelIterator( try (Closeable i

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-15 Thread via GitHub
danielcweeks commented on PR #7844: URL: https://github.com/apache/iceberg/pull/7844#issuecomment-1858369750 One minor comment but otherwise looks good to me. @nastra thoughts? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-12-15 Thread via GitHub
findepi commented on PR #7844: URL: https://github.com/apache/iceberg/pull/7844#issuecomment-1857726420 cc @danielcweeks -- 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

Re: [PR] Core: Add param to limit manifest parallel reader queue size [iceberg]

2023-11-30 Thread via GitHub
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 ignore