Skip to content

Commit

Permalink
Correctly consider negative max idle/max total sizes as unbounded asy…
Browse files Browse the repository at this point in the history
…nc pool size #1953
  • Loading branch information
mp911de committed Jan 4, 2022
1 parent e47d82e commit dc18d64
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 17 deletions.
42 changes: 28 additions & 14 deletions src/main/java/io/lettuce/core/support/BoundedAsyncPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ CompletableFuture<Void> 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++) {
Expand Down Expand Up @@ -188,8 +188,8 @@ CompletableFuture<Void> createIdle() {
return CompletableFuture.allOf(futures);
}

private int getAvailableCapacity() {
return getMaxTotal() - (getCreationInProgress() + getObjectCount());
private long getAvailableCapacity() {
return getActualMaxTotal() - (getCreationInProgress() + getObjectCount());
}

@Override
Expand Down Expand Up @@ -243,7 +243,7 @@ private void acquire0(T object, CompletableFuture<T> res) {

long objects = (long) (getObjectCount() + getCreationInProgress());

if ((long) getMaxTotal() >= (objects + 1)) {
if ((long) getActualMaxTotal() >= (objects + 1)) {
makeObject0(res);
return;
}
Expand All @@ -256,7 +256,7 @@ private void makeObject0(CompletableFuture<T> res) {
long total = getObjectCount();
long creations = objectsInCreationCount.incrementAndGet();

if (((long) getMaxTotal()) < total + creations) {
if (((long) getActualMaxTotal()) < total + creations) {

res.completeExceptionally(POOL_EXHAUSTED);
objectsInCreationCount.decrementAndGet();
Expand Down Expand Up @@ -349,7 +349,7 @@ public CompletableFuture<Void> release(T object) {
return Futures.failed(NOT_PART_OF_POOL);
}

if (idleCount.get() >= getMaxIdle()) {
if (idleCount.get() >= getActualMaxIdle()) {
return destroy0(object);
}

Expand Down Expand Up @@ -377,7 +377,7 @@ private CompletableFuture<Void> return0(T object) {

int idleCount = this.idleCount.incrementAndGet();

if (idleCount > getMaxIdle()) {
if (idleCount > getActualMaxIdle()) {

this.idleCount.decrementAndGet();
return destroy0(object);
Expand Down Expand Up @@ -451,27 +451,37 @@ public CompletableFuture<Void> 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.
Expand All @@ -484,7 +494,7 @@ public int getMaxIdle() {
*/
public int getMinIdle() {

int maxIdleSave = getMaxIdle();
int maxIdleSave = getActualMaxIdle();
if (this.minIdle > maxIdleSave) {
return maxIdleSave;
} else {
Expand All @@ -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;
}
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/io/lettuce/core/support/BoundedPoolConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,17 @@ void shouldCreateMaintainMinMaxIdleObject() {
assertThat(pool.getObjectCount()).isEqualTo(2);
}

@Test
void shouldCreateUnboundedPool() {

BoundedAsyncPool<String> pool = new BoundedAsyncPool<>(STRING_OBJECT_FACTORY,
BoundedPoolConfig.builder().maxTotal(-1).build());

TestFutures.awaitOrTimeout(pool.acquire());

assertThat(pool.getObjectCount()).isEqualTo(1);
}

@Test
void shouldReturnObject() {

Expand Down

0 comments on commit dc18d64

Please sign in to comment.