Bumping this thread a bit. Cleaning up the pool in non-static cases should be a responsibility of the user. If they want a pool which is closed by a hook when the JVM exists they should explicitly "say" so, for example calling "newExitingWorkerPool".
This is a behaviour change in the API, so I think we need feedback from the community before proceeding with it. What are your thoughts? Thanks, Peter 冯佳捷 <laputafa...@gmail.com> ezt írta (időpont: 2024. szept. 13., P, 17:16): > Hi all, > > During the investigation of a metaspace memory leak issue in Flink > IcebergSource ( https://github.com/apache/iceberg/pull/11073 ), a > discussion with @pvary revealed that *ThreadPools.newWorkerPool* > currently registers a Shutdown Hook via ExitingExecutorService for all > created thread pools. While this ensures graceful shutdown of the pools > when the JVM exits, it might lead to unnecessary Shutdown Hook > accumulation, especially when the pool is explicitly closed within the > application's lifecycle. > > I propose to *modify ThreadPools.newWorkerPool to not register a Shutdown > Hook by default*. This would prevent potential issues where developers > might unintentionally register numerous Shutdown Hooks when using > ThreadPools.newWorkerPool for short-lived thread pools. > To retain the existing functionality for long-lived thread pools that > require a Shutdown Hook, I suggest introducing a new, more descriptive > function, such as *newExitingWorkerPool*. This function would explicitly > create thread pools that are registered with a Shutdown Hook. > > *This change might potentially impact users who rely on the implicit > Shutdown Hook registration provided by the current > ThreadPools.newWorkerPool implementation.* > I would like to gather feedback from the community regarding this proposed > change, especially regarding potential compatibility concerns. > > Best regards, > Feng Jiajie > >
