fix(inkless:switch): stage reassignments of switched partitions - #757
Draft
jeqo wants to merge 1 commit into
Draft
fix(inkless:switch): stage reassignments of switched partitions#757jeqo wants to merge 1 commit into
jeqo wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Stages replica reassignments for switched diskless partitions to prevent premature ISR admission.
Changes:
- Limits immediate reassignment to presumed born-diskless partitions.
- Adds staged completion and cancellation tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java |
Selects immediate or staged reassignment based on switch state. |
metadata/src/test/java/org/apache/kafka/controller/ReplicationControlManagerInklessTest.java |
Tests staged reassignment completion and cancellation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jeqo
force-pushed
the
jeqo/no-one-step-reassign-switched
branch
from
August 18, 2026 11:10
c3087ba to
c87adb1
Compare
jeqo
force-pushed
the
jeqo/no-one-step-reassign-switched
branch
2 times, most recently
from
August 18, 2026 11:46
59858de to
6be393d
Compare
changePartitionReassignment applied the target replica set in one step for any diskless topic, setting targetIsr = target.replicas(). For born-diskless that is safe: all data is in object storage, so a target replica is current on arrival. A partition switched from classic breaks that premise. Records below classicToDisklessStartOffset live only in the replicas' local logs, so a replica newly added by a reassignment entered ISR holding none of the classic prefix -- not a stale prefix, none -- and was immediately electable. Nothing gated the reassignment path on the switch state. Restrict the one-step path to born-diskless partitions. A switched partition now takes the staged path: the target enters as addingReplicas and earns ISR through AlterPartition once its catch-up fetcher reaches the seal, which applyLocalFollowersDelta already arms for a newly added replica of a switched topic. Operator-visible consequence: reassigning a switched topic is pending rather than instant, and listPartitionReassignments reports it in progress until the new replica catches up. That is the point -- the wait is the evidence. This closes the grow half only. A pure RF shrink has no adding replicas, so completeReassignmentIfNeeded finishes the reassignment in the same operation and replicas collapses to the target before maybePopulateTargetElr runs, leaving ELR outside the replica set exactly as before (measured: elr=[1,2] replicas=[0] for a [0,1,2] -> [0] shrink at minISR=2). That half is not diskless-specific -- classic topics escape it only because RF is fixed without KIP-860 allow-RF-change -- and needs Rule B restricted to the target replica set instead, so it is left alone here. Tests cover the staged grow through to completion; cancellation, which unwinds adding/removing state on a diskless partition for the first time; and the consolidating variant, which stages the same way because its follower reaches the seal on the classic fetcher before isReadyForConsolidation hands off, so the leader still observes the fetch state that expands ISR.
jeqo
force-pushed
the
jeqo/no-one-step-reassign-switched
branch
from
August 18, 2026 11:51
6be393d to
9082a19
Compare
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.
Follow-up to #754, as agreed in review.
Independent of #754 and #756 — different files, no ordering requirement.
changePartitionReassignment applied the target replica set in one step for any diskless topic, setting targetIsr = target.replicas(). For born-diskless that is safe: all data is in object storage, so a target replica is current on arrival.
A partition switched from classic breaks that premise. Records below classicToDisklessStartOffset live only in the replicas' local logs, so a replica newly added by a reassignment entered ISR holding none of the classic prefix -- not a stale prefix, none -- and was immediately electable. Nothing gated the reassignment path on the switch state.
Restrict the one-step path to born-diskless partitions. A switched partition now takes the staged path: the target enters as addingReplicas and earns ISR through AlterPartition once its catch-up fetcher reaches the seal, which applyLocalFollowersDelta already arms for a newly added replica of a switched topic.
Operator-visible consequence: reassigning a switched topic is pending rather than instant, and listPartitionReassignments reports it in progress until the new replica catches up. That is the point -- the wait is the evidence.
Only growing and moving are fixed here. An RF shrink has nothing to catch up, so it still completes immediately and can leave ELR pointing at brokers that are no longer replicas (measured: elr=[1,2] with replicas=[0]). That is the existing I2 gap in ELR reconciliation rather than anything diskless-specific, so it needs its own fix.
Tests cover the staged grow through to completion, and cancellation, which unwinds adding/removing state on a diskless partition for the first time.
Consolidating switched partitions are out of scope and not yet safe: their follower evicts from the classic fetcher without waiting for ISR (ReplicaFetcherThread isConsolidatingPartition short-circuit), so the leader never records the catch-up and a staged reassignment would not complete. That needs a broker-side fix first.