FrankChen021 commented on PR #20403: URL: https://github.com/apache/druid/pull/20403#issuecomment-5789579501
FYI, #20273 currently modifies the same two files as this PR (`ResourcePool.java` and `ResourcePoolTest.java`), and a Git merge simulation confirms textual conflicts in both. I do not think #20273 makes this fix redundant. Its adaptive pool purges expired idle connections only during a subsequent `take(key)`. If a destination disappears and is never contacted again, its entry remains in the non-expiring `LoadingCache`, along with its eagerly initialized connections. This PR addresses the separate per-destination lifecycle by evicting the whole key and closing its holder. Suggestion: coordinate or stack the changes. If #20273 lands first, rebase this PR and port the expiration/removal-listener behavior onto `LoadingCache<K, PooledResources<V>>`, ensuring that an evicted `PooledResources` is closed. It would also be useful to add a direct endpoint-churn test: eagerly initialize endpoint A, let it expire, access endpoint B to trigger cache maintenance, and assert that A's resources are closed and its entry is removed. That test would distinguish whole-destination eviction from #20273's same-key connection shrinking. The `expireAfterAccess` overload flagged by CodeQL should also be updated while resolving this. -- 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]
