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

Warm the internalPool upon initialization of the JedisPool #1828

Closed
tmack8001 opened this issue Jul 6, 2018 · 2 comments · Fixed by #2521
Closed

Warm the internalPool upon initialization of the JedisPool #1828

tmack8001 opened this issue Jul 6, 2018 · 2 comments · Fixed by #2521

Comments

@tmack8001
Copy link

Expected behavior

I'd expect the behavior of creating a GenericConnectionPool to also warm the connection pool based on the configuration of the pool itself (minIdle, etc). This would take place by calling the following method upon initialization of the connection pool:

this.internalPool.preparePool();

This method internally will attempt to ensure that the correct minIdle instances are available for threads to consume and use at the point the pool is first configured. One approach could be to make this optional via a specific pool parameter/argument to say "warmPool: true | false"

Actual behavior

The GenericObjectPool is created with the correct pool configurations, but the pool is never warmed. The logic within GenericObjectPool itself doesn't warm the pool itself upon creation this is up to the user of the object pool instance to maintain.

Steps to reproduce:

Redis / Jedis Configuration

Jedis version: master or 2.9.0 (latest released version)

Redis version: N/A

Java version: N/A

@tmack8001 tmack8001 changed the title Upon Initialization of the JedisPool the pool isn't warmed Warm the internalPool upon initialization of the JedisPool Jul 6, 2018
@sazzad16
Copy link
Collaborator

sazzad16 commented Jul 8, 2018

@tmack8001 You are welcome to craft a PR, preferably with proper tests.

@tmack8001
Copy link
Author

@sazzad16 I have a change locally that seems to do what I intend and will offer up a PR by EOW

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants