maswin commented on PR #20403: URL: https://github.com/apache/druid/pull/20403#issuecomment-5916619867
> 1. Rebase. Both files conflict. The expiration and removal listener need to move onto LoadingCache<K, PooledResources> and close each evicted PooledResources. Done. > 2. Don't evict a destination while its connections are lent out (P1 in the review). Access time is only updated in take(). With the defaults (unusedConnectionTimeout PT4M, readTimeout PT15M), a long query can get its destination evicted mid-flight. The next take() then builds a fresh pool for that destination, which breaks the per-destination numConnections limit. Checking getUsedCount() == 0 before closing, or refreshing access time on giveBack, would fix it. Guava can't veto an expiry. When a take() hits an expired entry, it loads a new holder before the removal listener runs. So a check on getUsedCount() == 0 in the listener comes too late. Each pool entry now tracks when it was last taken or returned, and a sweep from take() closes one only if nothing is on loan and it has sat idle for 10× unusedConnectionTimeout. That way a long-running query never loses its pool, and take() just retries if it races with an eviction. > 3. Close expired entries on shutdown (P2). close() should run cleanUp() or invalidateAll() first, so expired entries aren't skipped. Since not using expireAfterAccess anymore, asMap would return all entries and closes every entry, including keys that are abandoned but not yet swept. > 4. Fix the CodeQL warning by using the Duration overload of expireAfterAccess. Not using expireAfterAccess anymore -- 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]
