Repository navigation
Stage CHK scale-up so 1→3 does not create all Raft members at once - #2087
Merged
Merged
Conversation
alex-zaitsev
force-pushed
the
fix/chk-staged-scale-up
branch
from
September 30, 2026 17:21
ffb6ba9 to
9bfc2c0
Compare
alex-zaitsev
marked this pull request as ready for review
October 1, 2026 07:03
Publish one new keeper at a time and wait Ready before the next STS, instead of writing the full raft_configuration and creating every replica after a fixed sleep. Co-authored-by: Cursor <cursoragent@cursor.com>
Do not create a new Keeper STS if raft_configuration was not published, and do not expand membership when STS existence is unknown. Restrict staging to 1→N / 2→3 and require the e2e join to become Ready. Co-authored-by: Cursor <cursoragent@cursor.com>
Identify 1→N / 2→3 joiners by topology so an interrupted Ready wait still serializes the next STS after status becomes Same. Do not publish raft_configuration until immediately before STS create, and do not honor readiness:false for that gate. Co-authored-by: Cursor <cursoragent@cursor.com>
alex-zaitsev
force-pushed
the
fix/chk-staged-scale-up
branch
from
October 21, 2026 09:41
ab45d0e to
d249758
Compare
Slach
reviewed
Oct 21, 2026
Slach
left a comment
Slach
left a comment
Collaborator
There was a problem hiding this comment.
A few inline notes. The main one is the retry path at worker-reconciler-chk.go:948: after an interrupted pass, the next joiner can still be created while the previous one isn't Ready.
Slach
self-requested a review
October 21, 2026 19:40
Slach
approved these changes
Oct 21, 2026
Slach
reviewed
Oct 21, 2026
| getErr = peerErr | ||
| return nil | ||
| } | ||
| if sts != nil && sts.Status.ReadyReplicas < 1 { |
Collaborator
There was a problem hiding this comment.
P1 — the deferral is unbounded, and every pass of it is expensive.
If h1 never becomes Ready (Keeper doesn't pick up the new raft_configuration live, bad image, PVC Pending), h2 is deferred on every pass, forever:
- each pass costs
membershipSettleDelay(30s) + ConfigMap writes, then a fixed 5s requeue that bypasses the rate limiter. With the default single CHK worker, other CHKs starve; reconcileCRpushes[RaftQuorumUnsafe] deferred host disruption...on every pass (worker-reconciler-chk.go:133) with no dedupe. After ~10 passes,status.errorsholds only that line, and it names the wrong reason.
Bound it (like quorumDisruptWaitTimeout), give it its own status reason with a deduped push, and show in e2e that h1 becomes Ready while h0 is never restarted.
Slach
approved these changes
Oct 22, 2026
This reverts commit abda7ac.
Member
Author
|
#2087 (comment) will be addressed in a separate PR |
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
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.
Summary
raft_configurationuntil the previous join is Ready, so the operator never creates every new StatefulSet in one pass.test_020005_2fail-fast:sts_count <= ready_count + 1on the existing test-052 manifests.Not in this PR (still #2041):
/keeper/config+mntrbarriers, scale-down purge,start_as_follower,rqld, lease election.Test plan
go test -mod=mod ./pkg/controller/chk/ ./pkg/apis/common/types/test_020005_2(fail-fast 1→3 invariant)test_020005(happy-path 1→3→1 + CHI replication)