osscm commented on PR #15492: URL: https://github.com/apache/iceberg/pull/15492#issuecomment-4124556876
> Thanks for the PR @osscm! I share some of @svalaskevicius concerns. Could you explain why we need this solution over #15312? IMO - Seems to be the root of the accumulation is not in the static pools that ThreadPools.java owns — those are singletons. The accumulation happens here: `RESTSessionCatalog.initialize()` → `OAuth2Manager.catalogSession()` → `new AuthSessionCache(...)` → `ThreadPools.newExitingWorkerPool(name + "-auth-session-evict", 1)` - Every `RESTSessionCatalog` construction walks that chain and registers a new permanent JVM hook. After `RESTSessionCatalog.close()`, the executor is shut down but the Guava hook is never removed. - #15312's shutdownThreadPools() manages pools stored as fields inside` ThreadPools.java`. Once newExitingWorkerPool() returns the executor to `AuthSessionCache`, ThreadPools.java has no reference to it. `setDefaultShutdownHook(false)` in #15312 would only suppress the static pool hooks — the AuthSessionCache eviction executors would still go through MoreExecutors.getExitingExecutorService() and accumulate hooks. - The registry approach in this PR is what closes that gap: every pool created via `newExiting*` is tracked centrally, and AuthSessionCache.close() calls ManagedThreadPools.remove() to deregister it immediately rather than waiting for JVM exit. -- 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]
