rdblue commented on PR #15312:
URL: https://github.com/apache/iceberg/pull/15312#issuecomment-4348265412

   @pvary, let me outline my assumptions so that my rationale is clear. First, 
I'm assuming that we are only worrying about the exiting pools. The other pools 
should have their own lifecycle management. The non-exiting pools are what I 
would expect `FileIO` instances to use, unless the pools are shared. Next, I 
see `newExistingScheduledPool` has 2 outside uses, `S3FileIO` and `GCSFileIO`, 
and `newExitingWorkerPool` has 3 outside uses in `HadoopFileIO`, 
`RESTMetricsReporter`, and `AuthSessionCache`. 5 uses is not a huge issue, but 
this could be a larger problem if the actual issue is that `FileIO` don't 
properly manage pool lifecycles.
   
   However, I don't think the `FileIO` situation has much influence over the 
decision here. This PR proposes that we add:
   1. A way to remove shutdown hooks from all thread pools
   2. A global shutdown for all thread pools created by `ThreadPools`
   
   I think those are both poor solutions. For the first one, if you don't want 
a threadpool that is configured like the static pools, the right answer is to 
use your own threadpool and manage its lifecycle. Customizing the lifecycle of 
the static pools opens the possibility of misconfiguration. The reasonable way 
to allow using your own threadpool (and not worrying about the shutdown hooks) 
is to lazily start the static pools. But if you can already shut down the 
static pools, then we don't really need even that.
   
   For the second one, I think it is a bad idea to have a global shutdown for 
the static pools, let along all of the pools created by the factory methods. 
This is a drastic behavior change that can be misused. It may allow us to avoid 
fixing pools created by `FileIO`, but we should not be creating dangerous ways 
to work around the right solution. And if I understand correctly, the problem 
you're saying we need to solve is that the pools created for `FileIO` have 
inconsistently managed lifecycles.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to