Repository navigation
[release-2.16] vitessshard: compute Idle when the shard record is missing (#831) - #851
Merged
mcrauwel merged 2 commits intoOct 26, 2026
Merged
Conversation
…sing Backport of #831 onto release-2.16 so a missing shard record after Reshard complete still computes Idle from serving partitions. Signed-off-by: Matthias Crauwels <matthias.crauwels@planetscale.com>
mcrauwel
marked this pull request as ready for review
October 22, 2026 15:35
mattlord
requested review from
arthurschreiber,
mattlord and
mhamza15
and
a balanced review from Copilot
October 22, 2026 15:37
There was a problem hiding this comment.
🟡 Changes recommended
A deployed cell disappearing from topology enumeration can allow shard deletion without verifying that cell’s serving references.
1 open issue
What changed in this PR
Backports the shard-cleanup fix to release-2.16 so obsolete shards can become idle after Reshard complete removes their topology records.
Changes:
- Compute idleness from serving topology when a shard record is missing, while keeping primary-related status unknown.
- Require readable keyspace and cell information before allowing cleanup.
- Add regression tests and delay end-to-end cleanup until the controller observes the missing record.
| File | Description |
|---|---|
test/endtoend/utils.sh |
Adds a shard-status polling helper. |
test/endtoend/upgrade_test.sh |
Waits for missing-record observation before removing old partitioning. |
pkg/controller/vitessshardreplication/init_shard_primary_test.go |
Tests that unknown primary status blocks initialization. |
pkg/controller/vitessshard/reconcile_topo.go |
Adds missing-record idleness checks against serving topology. |
pkg/controller/vitessshard/reconcile_topo_test.go |
Tests cleanup, repeated reconciliations, and topology-read failures. |
Review effort: Balanced
| // first. Only a cell that exists but has no SrvKeyspace for this keyspace | ||
| // counts as not serving. | ||
| func shardServingCells(ctx context.Context, ts *topo.Server, keyspace, shard string) ([]string, error) { | ||
| cells, err := ts.GetCellInfoNames(ctx) |
Collaborator
There was a problem hiding this comment.
This is valid, but it is not specific to this backport: the code is identical on main and release-2.17, and the path where the shard record still exists has the same gap via GetShardServingCells, which predates #831. I reproduced it on both paths with memorytopo. To keep this backport identical to #831, the fix will go to main first and then be backported. Tracking it in #863.
mattlord
changed the title
mattlord
approved these changes
Oct 22, 2026
arthurschreiber
approved these changes
Oct 26, 2026
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.
Backport of #831 onto
release-2.16(operator v2.16.* / Vitess v23).Fixes #830 for the v2.16 release line.
This is the full #831 change, including the review follow-ups:
Idlefrom serving partitions whenGetShardreturnsNoNodeHasMaster/ServingWritesUnknownwithout a shard recordCherry-pick applied cleanly;
reconcile_topo.goon this branch matched the pre-#831 main version. The e2e helper is inserted beforecheckPodStatusWithTimeoutbecausewaitForScheduledRolloutsToFinishdoes not exist on this line.Unit tests passed locally:
TestReconcileTopologyIdleWithoutShardRecord(all 7 subtests)TestReadyForPrimaryHasMasterUnknownNote: DCO-signed replacement for #849 (force-push blocked by repo ruleset).