maswin commented on code in PR #20403:
URL: https://github.com/apache/druid/pull/20403#discussion_r4147625666
##########
processing/src/main/java/org/apache/druid/java/util/http/client/pool/ResourcePool.java:
##########
@@ -62,8 +64,17 @@ public class ResourcePool<K, V> implements Closeable
public ResourcePool(final ResourceFactory<K, V> factory, final
ResourcePoolConfig config,
final boolean eagerInitialization)
{
- this.pool = CacheBuilder.newBuilder().build(
- new CacheLoader<>()
+ this.pool = CacheBuilder.newBuilder()
+
.expireAfterAccess(config.getUnusedConnectionTimeoutMillis(),
TimeUnit.MILLISECONDS)
Review Comment:
Agreed, and this is fixed. I dropped the cache-level expireAfterAccess.
Guava loads the replacement holder before the removal listener runs, so a
getUsedCount() check there comes too late.
Eviction is now tied to idleness instead. Each holder records its last take
or return, and the sweep closes a key only when getUsedCount() == 0 and it has
been idle for 10× unusedConnectionTimeout. A holder with anything on loan is
never evicted, however long the request runs. A later take() for that key gets
the same holder, so the limit holds.
testKeyWithResourceOnLoanIsNeverEvicted holds a resource well past the
timeout and checks that it isn't closed and is reused after it's returned.
--
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]
