Skip to content

Hand the master role over before replacing the master pod - #26

Merged
usiegj00 merged 1 commit into
mainfrom
26-promote-before-replacing-the-master
Sep 28, 2026
Merged

usiegj00 merged 1 commit into
mainfrom
26-promote-before-replacing-the-master

Conversation

@usiegj00

Copy link
Copy Markdown
Collaborator

Follow-up to #24, which was incomplete.

Requiring a ready replica before deleting the master removed the pathological case (59s masterless, nothing to promote) but left the ordering unchanged: the operator still deleted the master and elected a replacement on a later reconcile. On main that detection-and-promotion gap measured 11.5s on k8s 1.36.4, which is why main is currently red — the availability assertion caught it correctly.

1.35.8 1.36.4 1.37.0
before any fix 0s 0s 59s
#24 (ready-replica gate) 0s 11.5s 500ms
this PR ? ? ?

Change

Promote the best replica first, then leave the old master pod alone. Once demoted it is simply a replica with a stale revision, so the existing slave loop replaces it behind the same readiness quorum as any other replica. There is a master at every instant.

Reuses GetBestReplicaForPromotion and PromoteBestReplica, already used by the no-master branch. GetReplicaReplicationOffsets skips masters, so the selector is safe to call while a master is live.

Sentinel mode is untouched: it still deletes the master so sentinel runs the failover.

The e2e threshold goes back to 1 sample, since there should now be no window at all.

Requiring a ready replica before deleting the master removed the case where
there was nothing to promote, but not the outage itself: the operator still
deleted the master and elected a replacement on a later reconcile. That gap
measured up to 11.5s in CI and about 30s on a production cluster.

Promote the best replica first and leave the old master pod running. Once
demoted it is just a replica with a stale revision, so the existing slave
loop replaces it behind the same readiness quorum as any other replica.
There is a master at every instant.

This is the ordering redis documents for upgrades and that
spotahome#637 describes as a seamless rollout: replicas
first, then a failover, then the old master.

Sentinel mode is untouched and still deletes the master so sentinel runs
the failover.

Refs #23
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 53.33333% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
operator/redisfailover/checker.go 53.33% 6 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants