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

RoundRobinSocketAddressSupplier is not refreshing when RedisURI changes #1909

Closed
langchristian96 opened this issue Nov 2, 2021 · 4 comments
Labels
type: bug A general bug
Milestone

Comments

@langchristian96
Copy link

Bug Report

RoundRobinSocketAddressSupplier is not refreshing nodes IPs if only IPs change while nodeIds remain the same. Because of this the ConnectionWatchdog keeps retrying to connect with an old IP. (seems to be similar to what was reported under #1343 )

Current Behavior

  • Redis cluster with 6 nodes running under Kubernetes
  • Remove redis pods 2 by 2 until all of them get new IPs
  • After a while, the connection is remade and io.lettuce.core.cluster.RoundRobinSocketAddressSupplier#get is being called
  • Partitions supplier gives us the correct partition with same node Ids but different IPs (which is correct)
  • The following reset code inside the if block does not get executed
    if (!roundRobin.isConsistent(partitions)) { resetRoundRobin(partitions); }
    because the equals method from RedisClusterNode only compares the node ids
  • We end up reconnecting using the old ip

Expected behavior/code

If the nodes IPs change, we should do the round robin rebuild.

Environment

  • Lettuce version(s): 6.1.5.RELEASE
  • Redis version: 6.2.6

Possible Solution

Modify the equals method from RedisClusterNode to check the ip/uri as well

Additional context

dynamicRefreshSources was set to false and the DNS resolver used was DirContextDnsResolver

@langchristian96
Copy link
Author

On a second thought, comparing the URIs might not be the most correct approach since there can be aliases and it might affect other parts of the code. I think it might be better to go on another route and modify the reset conditions to check specifically if anything changed from an URI pov. Any thoughts on this?

@mp911de
Copy link
Collaborator

mp911de commented Nov 8, 2021

I'm happy for suggestions on how we can improve here.

@langchristian96
Copy link
Author

@mp911de, I've made the changes I suggested. Mind taking a look when you have time please?

@langchristian96
Copy link
Author

Managed to test the problematic scenario as well today and confirmed it works fine with the fix

@mp911de mp911de added this to the 6.0.9 milestone Dec 3, 2021
@mp911de mp911de added the type: bug A general bug label Dec 3, 2021
@mp911de mp911de changed the title RoundRobinSocketAddressSupplier is not refreshing nodes IPs RoundRobinSocketAddressSupplier is not refreshing when RedisURI changes Dec 3, 2021
mp911de pushed a commit that referenced this issue Dec 3, 2021
mp911de added a commit that referenced this issue Dec 3, 2021
Refactor BiFunction to isEqual BiPredicate. Return early if collection sizes do not match. Add tests and revise RedisClusterNode tests to verify equality and hashcode behavior.

Original pull request: #1912.
mp911de pushed a commit that referenced this issue Dec 3, 2021
mp911de added a commit that referenced this issue Dec 3, 2021
Refactor BiFunction to isEqual BiPredicate. Return early if collection sizes do not match. Add tests and revise RedisClusterNode tests to verify equality and hashcode behavior.

Original pull request: #1912.
mp911de pushed a commit that referenced this issue Dec 3, 2021
@mp911de mp911de closed this as completed Dec 3, 2021
mp911de added a commit that referenced this issue Dec 3, 2021
Refactor BiFunction to isEqual BiPredicate. Return early if collection sizes do not match. Add tests and revise RedisClusterNode tests to verify equality and hashcode behavior.

Original pull request: #1912.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
2 participants