-
Notifications
You must be signed in to change notification settings - Fork 24.4k
[Sentinel] set known sentinel instance port to 0 when original instance come online #8891
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
|
@hwware I think it may be a good idea to add a test for this scenario, maybe even with some kind of fuzzing (i.e. take up & down multiple sentinels). |
|
thank you @yossigo , i will think about how to add a test case for this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hwware Do we really need such a long delay here (and in the other locations)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yossigo we don't have to, the purpose of wait here is to make sure other sentinel get updated information from the pub/sub hello message, the interval is 2s. i think 3s should be enough, or what we can do is continusly waiting for other sentinel and as soon as the pub/sub message get updated we are good to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hwware I think it's worth taking the later approach, Sentinel tests are already pretty slow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yossigo I totally agree with you, and I will create a separated PR for this configuration. Thanks
7786134 to
2d0048c
Compare
…nstance_when_switch
|
This has been superseded by #9240, closing. |
Before this commit, in Sentinel mode, when the already known sentinel instance switch address, there is a chance making same sentinel instance being considered as two separate sentinel instances which may causing problem.
For example in the following scenario:
The root cause of this issue is when removing sentinel with same runid, we still need to invalidate other known sentinels with same ip/port.
https://github.com/redis/redis/blob/unstable/src/sentinel.c#L2874