Skip to content

Add helpers to Jedis pool #4366

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add helpers to Jedis pool #4366

wants to merge 5 commits into from
+45 −0

Conversation

@oscar-besga-panel
Copy link
Contributor

@oscar-besga-panel oscar-besga-panel commented Nov 20, 2025 •
edited
Loading

This mininal chage of code will allow to use Jedis connections without making a try/catch

Before

    try (Jedis jedis = jedisPool.getResource()) {
      //some code
       jedis.setnx("hello", "there");
      //some code
    }

After

jedisPool.withResource( (jedis) -> {
      //some code
       jedis.setnx("hello", "there");
      //some code
 });

Cleaner in my opinion

@jit-ci
Copy link

jit-ci bot commented Nov 20, 2025

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

@oscar-besga-panel
Copy link
Contributor Author

oscar-besga-panel commented Nov 20, 2025

retest

@ggivo
Copy link
Collaborator

ggivo commented Nov 21, 2025

@oscar-besga-panel
Did you consider JedisPooled? It is available starting with Jedis 4 and helps to avoid a try-with-resources block for each command.

https://redis.io/docs/latest/develop/clients/jedis/connect/#connect-with-a-connection-pool

ggivo
ggivo requested changes Nov 21, 2025
View reviewed changes
Copy link
Collaborator

@ggivo ggivo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general,

needs unit tests to verify the resource is returned to the pool, on success, on error.

src/main/java/redis/clients/jedis/JedisPool.java

public <K> K withJedisPoolGet(Function<Jedis, K> function) {
try (Jedis jedis = this.getResource()) {
return function.apply(jedis);
Copy link
Collaborator

@ggivo ggivo Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest :
Rename withJedisPoolGet → withResource

Copy link
Contributor Author

@oscar-besga-panel oscar-besga-panel Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See bellow

src/main/java/redis/clients/jedis/JedisPool.java Outdated
}
}

public void withJedisPoolDo(Consumer<Jedis> consumer) {
Copy link
Collaborator

@ggivo ggivo Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest :
Rename withJedisPoolDo → withResource

Copy link
Contributor Author

@oscar-besga-panel oscar-besga-panel Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is if the two methods are renamed equally this won't compile

        withResource(jedis -> jedis.set("c"));
        String x = withResource(jedis -> jedis.get("c"));

as it will trigger ambiguous method call in both lines.
Even if I like your idea, Java won't allow
(if you use a full body lambda it will make it, but that will reducefuture uses of the methods)

I'll suggest withResource and withResourceGet

@oscar-besga-panel
Copy link
Contributor Author

oscar-besga-panel commented Nov 21, 2025

@oscar-besga-panel Did you consider JedisPooled? It is available starting with Jedis 4 and helps to avoid a try-with-resources block for each command.

https://redis.io/docs/latest/develop/clients/jedis/connect/#connect-with-a-connection-pool

Yes, but I use JedisPool in my library for now.
(considering change as is Legacy but not deprecated)

JedisPooled is 99% the times better, except if you want to send multiple commands. then it will be getting/closing a pooled connection for every one.
Well you can use MULTI/EXEC or pipeline, thougth

@oscar-besga-panel
Copy link
Contributor Author

oscar-besga-panel commented Nov 21, 2025 •
edited
Loading

I'm considering extending JedisPooled to be compatible with JedisPool by making it return a Jedis object... but not for now; and I know it has serious challenges.
If it is to be done, in another PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ggivo ggivo ggivo requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@oscar-besga-panel @ggivo