Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BoundedAsyncPool doesn't work with a negative maxTotal #1181

Closed
sguillope opened this issue Nov 13, 2019 · 3 comments
Closed

BoundedAsyncPool doesn't work with a negative maxTotal #1181

sguillope opened this issue Nov 13, 2019 · 3 comments
Labels
type: bug A general bug
Milestone

Comments

@sguillope
Copy link

sguillope commented Nov 13, 2019

Bug Report

Current Behavior

When using the BoundedAsyncPool with a negative maxTotal, new pool objects are not created. Trying to acquire a new object will fail with io.lettuce.core.RedisException: java.util.NoSuchElementException: Pool exhausted

This can be reproduced by creating a pool with the following config:

BoundedPoolConfig.builder()
                .maxTotal(-1)
                .build();

Input Code

I've added 2 unit tests to BoundedAsyncPoolUnitTests based on the 5.2.x branch to demonstrate the failures:
https://github.com/lettuce-io/lettuce-core/compare/5.2.x...sguillope:bounded-async-pool-with-negative-max-total?expand=1

  • shouldCreateObjectWhenMaxTotalIsNegative fails with
io.lettuce.core.RedisException: java.util.NoSuchElementException: Pool exhausted

	at io.lettuce.core.LettuceFutures.awaitAll(LettuceFutures.java:88)
	at io.lettuce.core.LettuceFutures.awaitAll(LettuceFutures.java:46)
	at io.lettuce.test.Futures.get(Futures.java:98)
	at io.lettuce.test.Futures.get(Futures.java:116)
	at io.lettuce.core.support.BoundedAsyncPoolUnitTests.shouldCreateObjectWhenMaxTotalIsNegative(BoundedAsyncPoolUnitTests.java:72)
	at ...
Caused by: java.util.NoSuchElementException: Pool exhausted
	at io.lettuce.core.support.BoundedAsyncPool.acquire()(Unknown Source)
  • shouldCreateMinIdleObjectWhenMaxTotalIsNegative fails with
org.opentest4j.AssertionFailedError: 
Expecting:
 <0>
to be equal to:
 <2>
but was not.

Expected behavior/code

  • Min number of idle objects are created based on the minIdle config when the pool is initialised
  • New objects are created and returned by the pool when calling acquire()

Environment

  • Lettuce version(s): 5.2.1.RELEASE
  • Redis version: AWS Elasticache

Additional context

We found this issue after upgrading from SB 2.1 to 2.2.1 and to Spring Data Redis 2.2.1. We use the following Spring application.yml-based Redis config:

spring:
  redis:
    lettuce:
      pool:
        max-active: -1
        min-idle: 1
        max-idle: 10

When we upgraded our healthcheck (which include the redis component) started failing. Debugging found that java.util.NoSuchElementException: Pool exhausted was being thrown in RedisReactiveHealthIndicator.

@sguillope sguillope added the type: bug A general bug label Nov 13, 2019
@mp911de
Copy link
Collaborator

mp911de commented Nov 14, 2019

There's no support for -1 for maxTotal in BoundedPoolConfig. You're required to specify a max value, hence the name bounded pool. We have a utility (CommonsPool2ConfigConverter) that adapts a Commons Pool2 config to a BoundedPoolConfig. It would make sense to add a switch there to react to negative maxTotal settings and replace these with Integer.MAX_VALUE.

@sguillope
Copy link
Author

Yeah I figured it might have been intended that way. Unfortunately the pool configuration declared through Spring's properties is used for both sync and async pools.

For now we've changed our max-active to a very high integer value to make it work. Automatically falling back to Integer.MAX_VALUE would work for us.

Thanks

mp911de added a commit that referenced this issue Dec 26, 2019
Lettuce now adapts negative pool limits for the async pool to Integer.MAX_VALUE when using GenericObjectPoolConfig to align with the behavior of Commons Pool 2.
mp911de added a commit that referenced this issue Dec 26, 2019
Lettuce now adapts negative pool limits for the async pool to Integer.MAX_VALUE when using GenericObjectPoolConfig to align with the behavior of Commons Pool 2.
mp911de added a commit that referenced this issue Dec 26, 2019
Lettuce now adapts negative pool limits for the async pool to Integer.MAX_VALUE when using GenericObjectPoolConfig to align with the behavior of Commons Pool 2.
@mp911de mp911de added this to the 5.2.2 milestone Dec 26, 2019
@mp911de
Copy link
Collaborator

mp911de commented Dec 26, 2019

That's fixed now.

@mp911de mp911de closed this as completed Dec 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants