maytasm commented on PR #20403: URL: https://github.com/apache/druid/pull/20403#issuecomment-5867966495
@maswin Can you: 1. Rebase. Both files conflict. The expiration and removal listener need to move onto LoadingCache<K, PooledResources<V>> and close each evicted PooledResources. 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. 3. Close expired entries on shutdown (P2). close() should run cleanUp() or invalidateAll() first, so expired entries aren't skipped. 4. Fix the CodeQL warning by using the Duration overload of expireAfterAccess. Thanks! -- 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]
