Skip to content

Add support for static master-replica with Lettuce #46957

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
Closed
wants to merge 3 commits into from
Closed

Add support for static master-replica with Lettuce #46957

wants to merge 3 commits into from
+112 −2

Conversation

facewise
Copy link
Contributor

Hi.

I'd like to suggest support for Redis static Master/Replica auto-configuration.
This auto-configuration makes LettuceConnectionFactory use RedisStaticMasterReplicaConfiguration, e.g. AWS ElastiCache with Replicas.
I know it's cautious since Jedis doesn't support for static master-replica connection in Jedis, while Lettuce does.

Please feel free to review this suggestion and share your thoughts.

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

Thanks for the PR.

Given that the static master-replica support is specific to Lettuce, I am not sure that the proposed additions should be as central to the auto-configuration as they currently are. For example, the properties could be nested beneath spring.data.redis.lettuce to indicate that they're only supported with Lettuce.

It also doesn't feel quite right for RedisConnectionDetails to know about a Lettuce-specific feature. I'm not sure how to address this as I don't think we've faced this problem and I don't think we have an established pattern for dealing with it.

@facewise
Copy link
Contributor Author

I agree. Would it still be consideration if the properties go beneath spring.data.redis.lettuce and make RedisConnectionDetails unaware of this change?

@wilkinsona
Copy link
Member

I'd definitely like to see the code in that form please. IMO, it would be an improvement on the current proposal and something that we'd certainly consider.

@facewise facewise changed the title Support Redis static master-replica auto-configuration Support Lettuce static master-replica auto-configuration Sep 4, 2025
@facewise
Copy link
Contributor Author

facewise commented Sep 4, 2025

@wilkinsona
I've updated with the proposal. Please let me know about anything doesn't feel right.

snicoll
snicoll reviewed Sep 15, 2025
View reviewed changes
Copy link
Member

@snicoll snicoll left a comment

Choose a reason for hiding this comment

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

Thanks for the update. I've added a couple comments for you consideration.

I've also paged @mp911de for additional insights

...n/java/org/springframework/boot/data/redis/autoconfigure/LettuceConnectionConfiguration.java
if (staticMasterReplicaConfiguration != null) {
return new LettuceConnectionFactory(staticMasterReplicaConfiguration, clientConfiguration);
}

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the code move to a new Mode value. It feels odd that there is a if check right before we switch over all the supported modes.

...a-redis/src/main/java/org/springframework/boot/data/redis/autoconfigure/RedisProperties.java Outdated

private final Cluster cluster = new Cluster();

private final StaticMasterReplica staticMasterReplica = new StaticMasterReplica();
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we could name this Masterreplica. Groups should not use camel case as we don't want hyphens in them.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Sep 15, 2025
@snicoll
Copy link
Member

snicoll commented Sep 15, 2025

See spring-projects/spring-data-redis#3218 - @mp911de mentionned we could also get rid of the Masterreplicasub-group and just have those in the main lettuce namespace.

@facewise
Copy link
Contributor Author

@snicoll
I'm not sure how to move them in the main lettuce namespace.
Did you intent to configure static masterreplica when the spring.data.redis.lettuce.nodes are specified?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 18, 2025
@snicoll
Copy link
Member

snicoll commented Sep 18, 2025

Let's go ahead with that option and we can revisit based on the code.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Sep 18, 2025
@facewise facewise force-pushed the add-support-autoconfiguration-for-static-master-replica branch from 0ccaa99 to befbf86 Compare September 19, 2025 00:24
@snicoll snicoll removed the status: waiting-for-feedback We need additional information before we can continue label Sep 19, 2025
@snicoll snicoll self-requested a review September 19, 2025 08:20
@facewise facewise force-pushed the add-support-autoconfiguration-for-static-master-replica branch from befbf86 to 6175d90 Compare September 22, 2025 06:39
@facewise
Copy link
Contributor Author

@snicoll
I added a new Mode value but it feels little bit weird since Jedis doesn't support for static master-replica.

@snicoll
Copy link
Member

snicoll commented Sep 29, 2025 •
edited
Loading

Yeah I agree but silently ignoring it if the nodes property is set isn't great either. Let's see if @mp911de has an opinion about that.

@mp911de
Copy link
Member

mp911de commented Sep 29, 2025

it feels little bit weird since Jedis doesn't support for static master-replica.

It's a limitation of Jedis and it is what it is, so rejecting unsupported options is likely the best we can do.

MASTER_REPLICA could be likely a better name for the new mode constant (dropping the static part) as it is easier to use. The aspect that it is stating (i.e. endpoints are not automatically updated by a e.g. Sentinel or AWS daemon polling the environment config) is likely less relevant (at least from my perspective).

@snicoll
Copy link
Member

snicoll commented Sep 30, 2025

@facewise FTR I am taking over as we need to figure out how to present this to the user.

@snicoll snicoll self-assigned this Sep 30, 2025
@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 30, 2025
@snicoll snicoll added this to the 4.x milestone Sep 30, 2025
@snicoll snicoll changed the title Support Lettuce static master-replica auto-configuration Add support for static master-replica with Lettuce Oct 1, 2025
@snicoll
Copy link
Member

snicoll commented Oct 1, 2025

I've fixed formatting, checkstyle and a test that was broken. @facewise please run the build (on the command line) going forward before submitting something.

Looking at the overall change, the SSL bundle can't be linked to the standalone config so I am looking at our options.

@snicoll snicoll added the status: blocked An issue that's blocked on an external project change label Oct 1, 2025
snicoll pushed a commit to snicoll/spring-boot that referenced this pull request Oct 1, 2025
@facewise @snicoll
Add support for static master-replica with Lettuce
5def410 
See spring-projectsgh-46957

Signed-off-by: 용현 <[email protected]>
snicoll added a commit to snicoll/spring-boot that referenced this pull request Oct 1, 2025
@snicoll
Polish "Add support for static master-replica with Lettuce"
3ab0d67 
See spring-projectsgh-46957
@snicoll snicoll removed the status: blocked An issue that's blocked on an external project change label Oct 2, 2025
@snicoll snicoll modified the milestones: 4.x, 4.0.0-RC1 Oct 3, 2025
snicoll pushed a commit that referenced this pull request Oct 3, 2025
@facewise @snicoll
Add support for static master-replica with Lettuce
c280fde 
See gh-46957

Signed-off-by: 용현 <[email protected]>
snicoll added a commit that referenced this pull request Oct 3, 2025
@snicoll
Polish "Add support for static master-replica with Lettuce"
fbcc1fd 
See gh-46957
@snicoll snicoll closed this in 911578e Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snicoll snicoll Awaiting requested review from snicoll

Assignees

@snicoll snicoll

Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
4.0.0-RC1
Development

Successfully merging this pull request may close these issues.
5 participants
@facewise @wilkinsona @snicoll @mp911de @spring-projects-issues