pvary commented on PR #11073:
URL: https://github.com/apache/iceberg/pull/11073#issuecomment-2373402484
> @fengjiajie I think Ryan had a good point in the email thread that we
probably shouldn't be using the `ThreadPools.newWorkerPool()` with explicit
lifecycle management. I think we can switch the usage directly
`Executors.newFixedThreadPool` without adding
`ThreadPools.newNonExitingWorkerPool`.
Thanks @stevenzwu for chiming in!
What do you think we should do with the other occurrences where we use the
`ThreadPools.newWorkerPool()` incorrectly:
Here is the list:
- CountersBenchmark.defaultCounterMultipleThreads (core module)
- BaseDistributedDataScan.newMonitorPool (core module)
- FlinkInputFormat.createInputSplits (flink module)
- IcebergInputFormat.getSplits (flink module)
Incorrect, but typically called only once in the JVM lifecycle:
- TableMigrationUtil.migrationService - the pool management is abandoned,
and nothing prevents multiple pool creations (data module)
- IcebergCommitter<init> (flink module)
- IcebergFilesCommitter.open (flink module)
- IcebergSource.planSplitsForBatch (flink module)
- StreamingMonitorFunction.open (flink module)
- ContinuousSplitPlannerImpl<init> (flink module)
- Coordinator<init> - Kafka coordinator - I'm not sure that this belongs to
here (kafka-connect)
And here is the code we need to duplicate:
```
public static ExecutorService newNonExitingWorkerPool(String namePrefix,
int poolSize) {
return Executors.newFixedThreadPool(
poolSize,
new ThreadFactoryBuilder().setDaemon(true).setNameFormat(namePrefix
+ "-%d").build());
}
```
I would definitely create a util method for it. Minimally in the Flink
module.
OTOH this should be fixed in the other modules (code/data/kafka-connect),
and that points me to create a static method in a more central place. WDYT?
Thanks,
Peter
--
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
