Skip to content

Consider Lettuce instead of Jedis as default Redis driver dependency #10480

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

Closed
mp911de opened this issue Oct 2, 2017 · 9 comments
Closed

Consider Lettuce instead of Jedis as default Redis driver dependency #10480

mp911de opened this issue Oct 2, 2017 · 9 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@mp911de
Copy link
Member

mp911de commented Oct 2, 2017

Lettuce is the preferred Redis driver for Spring Data Redis and Spring Session supporting the most recent Redis versions. We should consider whether to replace Jedis with the Lettuce driver in spring-boot-starter-data-redis. There are a couple of aspects that play into this request:

  • Lettuce comes with a friendlier performance profile than Jedis.
  • The arrangement with Jedis suffers from changes in the Jedis core development. Features of newer Redis versions are not supported.
  • Requests for a new release were not completed in a timely manner.

On the other side, Lettuce 5 requires Java 8, netty 4.1 and Project Reactor 3.1. It no longer requires Google Guava.

Related issues:

/cc @christophstrobl @rwinch @vpavic

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 2, 2017
@wilkinsona
Copy link
Member

I'm now in favour of making this switch in 2.0.

@wilkinsona wilkinsona added for: team-attention An issue we'd like other members of the team to review priority: normal type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 2, 2017
@philwebb
Copy link
Member

philwebb commented Oct 3, 2017

3 x 👍 so I guess we're all in agreement.

@snicoll snicoll removed the for: team-attention An issue we'd like other members of the team to review label Oct 3, 2017
@snicoll snicoll added this to the 2.0.0.M5 milestone Oct 3, 2017
@snicoll snicoll self-assigned this Oct 3, 2017
@snicoll
Copy link
Member

snicoll commented Oct 4, 2017

This has the interesting side effect of not requiring a dedicated "reactive" starter for redis anymore so it will be removed as part of this change.

@snicoll snicoll closed this as completed in 9f998d6 Oct 4, 2017
@vpavic
Copy link
Contributor

vpavic commented Oct 4, 2017

Hmm, doesn't the starter also provide Project Reactor?

For example spring-boot-starter-data-cassandra-reactive only provides Reactor on top of what spring-boot-starter-data-cassandra provides so this change makes things inconsistent with Cassandra.

@snicoll
Copy link
Member

snicoll commented Oct 4, 2017

Lettuce already uses reactor. The reason for a separate starter is because the driver is different (as for MongoDB). I don't think we should duplicate the starter only for that reason.

@vpavic
Copy link
Contributor

vpavic commented Oct 4, 2017

Yes, I'm aware, but if I'm not mistaken reactive dependencies are optional to Lettuce. So the net effect is that the user needs to provide additional dependencies on top of spring-boot-starter-data-redis to make things work while with Cassandra there's spring-boot-starter-data-cassandra-reactive that takes care of that.

I don't have a preference whether spring-boot-starter-data-cassandra-reactive should be removed as well or should spring-boot-starter-data-redis-reactive be reinstated, it's just that thing seems inconsistent.

@snicoll
Copy link
Member

snicoll commented Oct 4, 2017

The dep is mandatory so nothing is really lost. I wonder about the naming inconsistency now. Let's reopen and see what the rest of the team thinks.

@snicoll snicoll reopened this Oct 4, 2017
@snicoll snicoll added the for: stackoverflow A question that's better suited to stackoverflow.com label Oct 4, 2017
philwebb added a commit that referenced this issue Oct 4, 2017
Update `LettuceConnectionConfiguration` so that `commons-pool2` can
be an optional dependency.

See gh-10480
@philwebb philwebb added for: team-attention An issue we'd like other members of the team to review and removed for: stackoverflow A question that's better suited to stackoverflow.com labels Oct 4, 2017
@vpavic
Copy link
Contributor

vpavic commented Oct 6, 2017

The dep is mandatory so nothing is really lost.

My mistake, I've misread the POM. Sorry for the noise.

@snicoll snicoll removed their assignment Oct 9, 2017
@philwebb
Copy link
Member

philwebb commented Oct 11, 2017

I think the naming is fine. We have spring-boot-starter-data-redis which provides both reactive and non-reactive support (as I understand it). The reactor-core dependency is mandatory, even in a non-reactive user application.

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

No branches or pull requests

6 participants