Repository navigation
Sentinel: re-resolve the primary when it is demoted on a healthy connection - #3281
Open
rebaseandpanic wants to merge 1 commit into
Open
rebaseandpanic wants to merge 1 commit into
rebaseandpanic wants to merge 1 commit into
Conversation
… connection A sentinel-managed connection notices an in-place demotion of its primary (INFO replication reports role:slave) but only re-elects among the nodes it already knows. When sentinel promoted a node at an address the client has never seen and the +switch-master announcement was missed, there is no primary among them and nothing asks sentinel again: writes fail with "No connection (requires writable - not eligible for replica)" until restart. The sentinel connection's heartbeat now checks, from local state only, whether each managed connection's sentinel-reported primary is connected and reporting itself as a replica (or was never added). If so, a recovery attempt runs off the heartbeat thread, at most one per managed connection: - it asks sentinel first; no answer: retry on a later heartbeat; - sentinel names a node held as a connected replica (sentinel has not failed over yet, or our view of that node's role is stale): re-read that node's role with one INFO replication, rather than reconfiguring every node once per heartbeat for as long as sentinel lags (where INFO cannot be sent, fall through to the switch rather than ask sentinel forever); - otherwise switch, as +switch-master would have. An attempt is skipped while the managed connection is already reconfiguring, or once either multiplexer is disposed; disconnected primaries are left to the existing reconnect timer. The heartbeat scan also forgets disposed children, as the +switch-master handler does. Tests: an in-process sentinel (InProcessSentinelServer) and a standalone SetReplicaOf on the toy server. The demotion-to-unknown-address test fails without the fix (no primary at all) and passes with it; a second test holds sentinel on the demoted node and checks that sentinel is still polled but the connection is not reconfigured on every heartbeat, then follows sentinel once it catches up. Refs StackExchange#1891, StackExchange#2487, StackExchange#1670
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scenario
The client uses Sentinel mode (
ServiceName) and is connected to primary A.+switch-mastermessage never reaches the client.Today the client sees the demotion (
INFO replicationreportsrole:slave) but only re-elects among the nodes it knows; all of them are replicas, and sentinel is not asked again. Writes fail until restart:#3130 fixed the case where the new primary is already known; this is the unknown-address case it left untouched. We hit this class of failure in production (Valkey + Sentinel on Kubernetes).
Fix
On the sentinel connection's heartbeat (every second), using local state only: if the primary sentinel last reported is connected but reports itself as a replica, run one recovery attempt in the background:
INFO(no full reconfigure);+switch-masterwould have.Skipped while a reconfigure is running, after dispose, or when disconnected (the existing reconnect timer handles that). No public API change, no periodic sentinel polling. This goes slightly beyond the sentinel client guidelines ("Handling reconnections"), which rely on sentinel disconnecting the demoted node's clients.
Tests
In-process sentinel (
InProcessSentinelServer) plusSetReplicaOfon the toy server:Also checked against real Valkey 8.1 + Sentinel in docker: the client switched ~1 s after noticing the demotion.
Refs #1891, #2487, #1670
Checklist