fix(cluster): derive shard membership from topology, not pod ordinal - #37
Conversation
Changing spec.replicasPerShard on a running cluster-mode RedisCluster reinterpreted existing primaries as replicas, because shard membership was computed as index / (1 + replicasPerShard). The operator then sent CLUSTER REPLICATE to nodes that own slots and looped on HTTP 500 (#36). Shard membership now follows the live topology: a slot owner is the primary of the shard whose range it serves, replicas follow the primary they replicate from (new primaryNodeID in /v1/status), and only empty pods are placed. Pod labels and status.shards are derived the same way. Also fixed along the way: - the instance manager replaced its whole instancesStatus entry and so wiped the operator's nodeID/slotsServed/clusterState fields each tick; it now updates only the fields it observes - CLUSTER REPLICATE against a pod that has not yet learned about its primary returns 409 and is retried instead of logged as an error - instance-manager HTTP errors include the response body Fixes #36
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3557494a1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for shardIndex := 0; shardIndex < shardCount; shardIndex++ { | ||
| podName := layout.primaryOf[shardIndex] | ||
| if podName == "" { | ||
| return false, nil | ||
| } | ||
| desiredPrimaryPods = append(desiredPrimaryPods, podName) |
There was a problem hiding this comment.
When replicasPerShard is reduced after a Redis failover or on a legacy contiguous layout, layout.primaryOf can select a high-ordinal pod that falls outside the new desired pod set. Resharding then treats that pod as the desired target and leaves its slots in place, but reconcilePods subsequently deletes the highest ordinals; cluster mode also clears status.currentPrimary, so its primary safeguard cannot help. For example, scaling a six-pod cluster to three after pod 3 takes over shard 0 deletes pod 3 with its slots, leaving the shard unavailable. Limit targets to pods that will survive the downscale, or migrate slots off doomed primaries before deletion.
Useful? React with 👍 / 👎.
Scale-down deletes the highest ordinals. After a Redis failover, or on the old contiguous layout, a slot owner can live there, and lowering spec.replicasPerShard would delete it with its slots. The layout planner now marks owners beyond the desired pod count as doomed and picks a surviving pod as the shard primary: a surviving replica of the owner, else a free survivor, else a replica borrowed from the largest shard. Reshard hands the shard over before deletion: the heir replicates from the doomed owner, then takes over with CLUSTER FAILOVER once its link is up. Pod deletion waits on reshard as before.
The unreleased notes and the cluster handover section of the service contract were one long clause after another. Split them into the actual steps. Drop comments that restated the next line.
The planner comments restated the next line. The service contract handover section is one table row. Changelog is the user-facing summary, not a second design doc.
Bump the chart and install docs to 0.2.7 and record the cluster-mode topology membership fix (#37) in the changelog ahead of tagging v0.2.7.
1 participant
Fixes #36.
Shard membership used to be
index / (1 + replicasPerShard). Raise replicas from 0 to 1 on a 3-shard cluster and pod 1, primary of s1, becomes the expected replica of s0. The operator sendsCLUSTER REPLICATE, Redis refuses because the node owns slots, and the loop sits on HTTP 500. The same formula made a Redis failover look like a misplacement, so reshard tried to move slots back to the old primary.Membership now comes from live topology in
cluster_layout.go. Slot owners are primaries of the range they serve. Replicas followprimaryNodeIDfrom/v1/status. Only empty pods get placed. Labels andstatus.shardsuse the same plan.Scale-down still deletes the highest ordinals, which can be slot owners after a failover. Those owners are marked doomed. A surviving heir replicates from them, then takes over with
CLUSTER FAILOVER. The old primary is fenced unready first; Redis stays up. Emergency fencing still stops Redis.Also: the instance manager no longer wipes operator-owned cluster fields each tick.
CLUSTER REPLICATEagainst an unknown primary is 409 and retries.Kind check with the issue manifest: 0→1→2→1 replicas stayed healthy, no slot moves, failover and label repair followed topology.
test/smoke/cluster_replica_scaling.pycovers doomed-primary scale-down and both fence types.Leftovers:
MIGRATEstill has no auth password. Scaled-down nodes are neverCLUSTER FORGET-ed. New clusters put primaries on pods0..shards-1.