From dc18d644914f42e210b90c563856c3260cf00957 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 4 Jan 2022 09:45:03 +0100 Subject: [PATCH] Correctly consider negative max idle/max total sizes as unbounded async pool size #1953 --- .../core/support/BoundedAsyncPool.java | 42 ++++++++++++------- .../core/support/BoundedPoolConfig.java | 6 +-- .../support/BoundedAsyncPoolUnitTests.java | 11 +++++ 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/src/main/java/io/lettuce/core/support/BoundedAsyncPool.java b/src/main/java/io/lettuce/core/support/BoundedAsyncPool.java index cec407dcbd..e50cb17b65 100644 --- a/src/main/java/io/lettuce/core/support/BoundedAsyncPool.java +++ b/src/main/java/io/lettuce/core/support/BoundedAsyncPool.java @@ -158,8 +158,8 @@ CompletableFuture createIdle() { return (CompletableFuture) COMPLETED_FUTURE; } - int totalLimit = getAvailableCapacity(); - int toCreate = Math.min(Math.max(0, totalLimit), potentialIdle); + long totalLimit = getAvailableCapacity(); + int toCreate = Math.toIntExact(Math.min(Math.max(0, totalLimit), potentialIdle)); CompletableFuture[] futures = new CompletableFuture[toCreate]; for (int i = 0; i < toCreate; i++) { @@ -188,8 +188,8 @@ CompletableFuture createIdle() { return CompletableFuture.allOf(futures); } - private int getAvailableCapacity() { - return getMaxTotal() - (getCreationInProgress() + getObjectCount()); + private long getAvailableCapacity() { + return getActualMaxTotal() - (getCreationInProgress() + getObjectCount()); } @Override @@ -243,7 +243,7 @@ private void acquire0(T object, CompletableFuture res) { long objects = (long) (getObjectCount() + getCreationInProgress()); - if ((long) getMaxTotal() >= (objects + 1)) { + if ((long) getActualMaxTotal() >= (objects + 1)) { makeObject0(res); return; } @@ -256,7 +256,7 @@ private void makeObject0(CompletableFuture res) { long total = getObjectCount(); long creations = objectsInCreationCount.incrementAndGet(); - if (((long) getMaxTotal()) < total + creations) { + if (((long) getActualMaxTotal()) < total + creations) { res.completeExceptionally(POOL_EXHAUSTED); objectsInCreationCount.decrementAndGet(); @@ -349,7 +349,7 @@ public CompletableFuture release(T object) { return Futures.failed(NOT_PART_OF_POOL); } - if (idleCount.get() >= getMaxIdle()) { + if (idleCount.get() >= getActualMaxIdle()) { return destroy0(object); } @@ -377,7 +377,7 @@ private CompletableFuture return0(T object) { int idleCount = this.idleCount.incrementAndGet(); - if (idleCount > getMaxIdle()) { + if (idleCount > getActualMaxIdle()) { this.idleCount.decrementAndGet(); return destroy0(object); @@ -451,27 +451,37 @@ public CompletableFuture closeAsync() { * checkout) at a given time. When negative, there is no limit to the number of objects that can be managed by the pool at * one time. * - * @return the cap on the total number of object instances managed by the pool. + * @return the cap on the total number of object instances managed by the pool. Unlimited max objects are capped at + * {@link Integer#MAX_VALUE Integer#MAX_VALUE}. * @see BoundedPoolConfig#getMaxTotal() */ public int getMaxTotal() { return maxTotal; } + private int getActualMaxTotal() { + return maxOrActual(maxTotal); + } + /** * Returns the cap on the number of "idle" instances in the pool. If {@code maxIdle} is set too low on heavily loaded * systems it is possible you will see objects being destroyed and almost immediately new objects being created. This is a - * result of the active threads momentarily returning objects faster than they are requesting them them, causing the number - * of idle objects to rise above maxIdle. The best value for maxIdle for heavily loaded system will vary but the default is - * a good starting point. + * result of the active threads momentarily returning objects faster than they are requesting them, causing the number of + * idle objects to rise above maxIdle. The best value for maxIdle for heavily loaded system will vary but the default is a + * good starting point. * - * @return the maximum number of "idle" instances that can be held in the pool. + * @return the maximum number of "idle" instances that can be held in the pool. Unlimited idle objects are capped at + * {@link Integer#MAX_VALUE Integer#MAX_VALUE}. * @see BoundedPoolConfig#getMaxIdle() */ public int getMaxIdle() { return maxIdle; } + private int getActualMaxIdle() { + return maxOrActual(maxIdle); + } + /** * Returns the target for the minimum number of idle objects to maintain in the pool. If this is the case, an attempt is * made to ensure that the pool has the required minimum number of instances during idle object eviction runs. @@ -484,7 +494,7 @@ public int getMaxIdle() { */ public int getMinIdle() { - int maxIdleSave = getMaxIdle(); + int maxIdleSave = getActualMaxIdle(); if (this.minIdle > maxIdleSave) { return maxIdleSave; } else { @@ -508,6 +518,10 @@ private boolean isPoolActive() { return this.state == State.ACTIVE; } + private static int maxOrActual(int count) { + return count > -1 ? count : Integer.MAX_VALUE; + } + enum State { ACTIVE, TERMINATING, TERMINATED; } diff --git a/src/main/java/io/lettuce/core/support/BoundedPoolConfig.java b/src/main/java/io/lettuce/core/support/BoundedPoolConfig.java index fffb7851e2..c798bcfffb 100644 --- a/src/main/java/io/lettuce/core/support/BoundedPoolConfig.java +++ b/src/main/java/io/lettuce/core/support/BoundedPoolConfig.java @@ -168,9 +168,9 @@ public Builder maxTotal(int maxTotal) { /** * Returns the cap on the number of "idle" instances in the pool. If {@code maxIdle} is set too low on heavily loaded * systems it is possible you will see objects being destroyed and almost immediately new objects being created. This is - * a result of the active threads momentarily returning objects faster than they are requesting them them, causing the - * number of idle objects to rise above maxIdle. The best value for maxIdle for heavily loaded system will vary but the - * default is a good starting point. + * a result of the active threads momentarily returning objects faster than they are requesting them, causing the number + * of idle objects to rise above maxIdle. The best value for maxIdle for heavily loaded system will vary but the default + * is a good starting point. * * @param maxIdle the cap on the number of "idle" instances in the pool. * @return {@code this} {@link Builder}. diff --git a/src/test/java/io/lettuce/core/support/BoundedAsyncPoolUnitTests.java b/src/test/java/io/lettuce/core/support/BoundedAsyncPoolUnitTests.java index 35fd2feb4d..fc72640c15 100644 --- a/src/test/java/io/lettuce/core/support/BoundedAsyncPoolUnitTests.java +++ b/src/test/java/io/lettuce/core/support/BoundedAsyncPoolUnitTests.java @@ -150,6 +150,17 @@ void shouldCreateMaintainMinMaxIdleObject() { assertThat(pool.getObjectCount()).isEqualTo(2); } + @Test + void shouldCreateUnboundedPool() { + + BoundedAsyncPool pool = new BoundedAsyncPool<>(STRING_OBJECT_FACTORY, + BoundedPoolConfig.builder().maxTotal(-1).build()); + + TestFutures.awaitOrTimeout(pool.acquire()); + + assertThat(pool.getObjectCount()).isEqualTo(1); + } + @Test void shouldReturnObject() {