Skip to content

Add lettuce Redis driver autoconfiguration. #5311

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

Conversation

mp911de
Copy link
Member

@mp911de mp911de commented Mar 2, 2016

Introduce an alternative autoconfiguration if the lettuce Redis driver is available. Add lettuce-specific configuration property options spring.redis.lettuce.shutdown-timeout to control the shutdown timeout of the lettuce driver. Add documentation for the properties, the supported drivers, and how to switch between drivers.

Jedis stays the default driver even if both libraries are on the classpath.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 2, 2016
@mp911de mp911de force-pushed the issue/sd-redis-lettuce-driver-autoconfiguration branch from f8a8c3d to d0b063f Compare March 3, 2016 07:17
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 6, 2016
@wilkinsona wilkinsona modified the milestone: 1.4.0.M3 Apr 6, 2016
@wilkinsona
Copy link
Member

I think this requires a bit more thought (and possibly some breaking changes).

We currently have some spring.redis.pool.* properties that are ignored if you are using Lettuce. I think that's likely to confuse people. They should perhaps be moved to spring.redis.jedis.pool. What do you think @snicoll?

I am not keen on LettuceRedisProperties being mapped to a subset of the namespace that's already been claimed by RedisProperties. I think it would be preferable to follow the approach taken in ServerProperties so we'd have a RedisProperties.Lettuce inner-class similar to the ServerProperties.Tomcat inner class.

@wilkinsona wilkinsona added the status: on-hold We can't start working on this issue yet label Apr 6, 2016
@snicoll
Copy link
Member

snicoll commented Apr 18, 2016

I reacted with 👍 so maybe you didn't get the notification. Note also that jedis does require commons-pool2 so this would be a good thing to add this as an alternative (even though it does need Guava)

@mp911de mp911de force-pushed the issue/sd-redis-lettuce-driver-autoconfiguration branch 5 times, most recently from 2c1448c to 347afe6 Compare May 3, 2016 13:04
@mp911de
Copy link
Member Author

mp911de commented May 3, 2016

Thanks for your feedback. I addressed your comments and pushed new commits, rebased to master.

@mp911de mp911de force-pushed the issue/sd-redis-lettuce-driver-autoconfiguration branch from 347afe6 to 9a8649f Compare May 30, 2016 06:42
@mp911de mp911de force-pushed the issue/sd-redis-lettuce-driver-autoconfiguration branch 2 times, most recently from a09992a to 99b6590 Compare September 5, 2016 08:33
@rwinch
Copy link
Member

rwinch commented Oct 25, 2016

I would really like to see this support too. The lettuce driver uses multiplexing which reduces the overall connections to Redis. This means the lettuce driver works much better with Spring Session. In fact, the Pivotal CLA tooling switched to Lettuce due to larger number of connections causing alerting on our Redis instance.

@wilkinsona wilkinsona added the for: team-attention An issue we'd like other members of the team to review label Oct 25, 2016
@snicoll snicoll removed the status: on-hold We can't start working on this issue yet label Oct 26, 2016
@philwebb philwebb added this to the 1.5.0 milestone Nov 2, 2016
@wilkinsona wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label Nov 2, 2016
@snicoll snicoll modified the milestones: 1.5.0 RC1, 1.5.0 Dec 1, 2016
@snicoll snicoll removed their assignment Dec 1, 2016
@snicoll snicoll modified the milestones: 2.0.0, 1.5.0 M1 Dec 16, 2016
@snicoll
Copy link
Member

snicoll commented Dec 16, 2016

Moving to 2.0 as Lettuce 4.x requires Java8

@mp911de mp911de force-pushed the issue/sd-redis-lettuce-driver-autoconfiguration branch 2 times, most recently from 4f8ceae to ee2cb13 Compare December 16, 2016 14:13
@snicoll snicoll added the status: on-hold We can't start working on this issue yet label Feb 7, 2017
@snicoll
Copy link
Member

snicoll commented Feb 7, 2017

This PR will be reworked by @mp911de once breaking changes in Kay have landed on master

Introduce an alternative autoconfiguration if the lettuce Redis driver is available. Add Lettuce-specific configuration property options "spring.redis.lettuce.shutdown-timeout" to control the shutdown timeout of the lettuce driver. Add documentation for the properties, the supported drivers, and how to switch between drivers.

Split client-specific properties from spring.redis.pool to spring.redis.jedis.pool and introduce spring.redis.lettuce namespace. Deprecate spring.redis.pool property.
@mp911de mp911de force-pushed the issue/sd-redis-lettuce-driver-autoconfiguration branch from ee2cb13 to 8ad8d37 Compare April 24, 2017 14:15
@snicoll snicoll self-assigned this May 2, 2017
@snicoll snicoll modified the milestones: 2.0.0.M1, 2.0.0.M2 May 2, 2017
snicoll pushed a commit that referenced this pull request May 2, 2017
Introduce an alternative autoconfiguration if the lettuce Redis driver is
available. Add Lettuce-specific configuration property options
"spring.redis.lettuce.shutdown-timeout" to control the shutdown timeout
of the lettuce driver. Add documentation for the properties, the
supported drivers, and how to switch between drivers.

Split client-specific properties from spring.redis.pool to
spring.redis.jedis.pool and introduce spring.redis.lettuce namespace.
Deprecate spring.redis.pool property.

See gh-5311
@snicoll snicoll closed this in e7efa8f May 2, 2017
snicoll added a commit that referenced this pull request May 2, 2017
…utoconfiguration

* pr/5311:
  Add missing tests
  Polish "Add Lettuce Redis driver autoconfiguration"
  Add Lettuce Redis driver autoconfiguration
@snicoll
Copy link
Member

snicoll commented May 2, 2017

Thank you so much @mp911de!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: on-hold We can't start working on this issue yet type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants