Skip to content

Conversation

@moticless
Copy link
Collaborator

When updating SENTINEL with master’s new password (command:
SENTINEL SET mymaster auth-pass some-new-password),
sentinel might still keep the old connection and avoid reconnecting
with the new password. This is because of wrong logic that traces
the last ping (pong) time to servers. In fact it worked fine until 8631e64
changed the condition to send ping. To resolve it with minimal risk,
let’s disconnect master and replicas once changing password/user.

Based on earlier work of: yz1509

@moticless moticless requested review from hwware and yossigo March 9, 2022 06:21
@moticless moticless changed the title Adding the fix along with TCL testing - draft Sentinel not try reconnect master after SENTINEL SET auth-pass Mar 9, 2022
@moticless moticless force-pushed the fix-sentinel-on-pwd-change branch from 7ef4bd7 to 590bcec Compare March 9, 2022 06:46
@yossigo yossigo changed the title Sentinel not try reconnect master after SENTINEL SET auth-pass Sentinel: fix no reconnect after auth-pass is changed. Mar 13, 2022
@yossigo yossigo merged commit a6bf509 into redis:unstable Mar 13, 2022
@yossigo yossigo added the release-notes indication that this issue needs to be mentioned in the release notes label Mar 13, 2022
enjoy-binbin added a commit to enjoy-binbin/redis that referenced this pull request Mar 13, 2022
We need to wait for `sentinelTimer` to kick in, and then
trigger the reconnect.

As for another change, i think we should better call
`server_set_password` before calling SENTINEL SET auth-pass.

Introduced in redis#10400
yossigo pushed a commit that referenced this pull request Mar 14, 2022
We need to wait for `sentinelTimer` to kick in, and then trigger the reconnect.

As for another change, we should better call `server_set_password` before calling SENTINEL SET auth-pass.

Fixes problem introeuced in #10400
@moticless moticless deleted the fix-sentinel-on-pwd-change branch April 3, 2022 06:33
@oranagra oranagra mentioned this pull request Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants