[KAFKA-20295] decommision (pre-unregister) controller patch - #70
Open
HelenMel wants to merge 3 commits into
Open
[KAFKA-20295] decommision (pre-unregister) controller patch#70HelenMel wants to merge 3 commits into
HelenMel wants to merge 3 commits into
Conversation
| throw new ControllerIdNotRegisteredException("Controller " + controllerId + | ||
| " is not registered, so it cannot be decommissioned."); | ||
| } | ||
| if (featureControl.isControllerId(controllerId)) { |
There was a problem hiding this comment.
This guard seems to only check the static controller.quorum.voters list via featureControl.isControllerId(controllerId), not the current live Raft voter set.
In a dynamic quorum that means a controller can still be a live voter even after being removed from static config, and this code would then decommission that still-live voter and skip it from controllerSupportedFeatures().
Can you validate against the current voter set here instead of only the static config?
Comment on lines
+89
to
+91
| "that is by design (see docs/operations/kraft.md). Once the cluster's " + | ||
| "metadata.version supports it, this automatically escalates into " + | ||
| "unregistering the controller instead."); |
There was a problem hiding this comment.
The help text here says that when metadata.version supports it, decommissioning will automatically escalate into a real unregister-controller operation.
Is this already implemented in this 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.
Goal: introduce a new operator action — decommission a controller — after which that
controller no longer participates in feature /
metadata.versionupgrade decisions. Physical removalof its registration is explicitly not part of decommissioning on 4.0.x–4.2.x (D6).
Terminology (read this first)
metadata.versionupgrade decisions. Its registration stays in the metadata image.UnregisterControllerRecord.metadata.version≥4.4-IV2)The two are not the same operation and this task does not backport unregistration. They are
sequential states of one lifecycle: a controller is decommissioned first, and once the cluster can
support it (MV ≥
4.4-IV2) a decommissioned controller may additionally be unregistered. Per D8 theescalation is automatic and server-side, so the operator keeps running one command.
1. Problem
A KRaft controller that has left the cluster keeps its
RegisterControllerRecordin__cluster_metadataforever — there is no unregister record, no fencing, no expiry. Registrationsare append-only and overwrite-by-id (
ClusterControlManager.replay():679doescontrollerRegistrations.put(...), neverremove). The stale entry is still consulted whenvalidating feature and
metadata.versionupgrades.3. Requirements
3.1 Functional
metadata.versionis as low as3.7-IV0, viakafka-cluster.sh decommission-controller --controller-id <id>.FeatureControlManagerno longer consults that controller, sokafka-features.sh upgradesucceeds.in decisions again. Intended behaviour (D7); free, because the RPC path rebuilds
Featuresfromthe request (
ClusterControlManager.java:469-475, evidence E9).CONTROLLER_ID_NOT_REGISTERED.representation this MV would write, the call succeeds and emits nothing (D11). On 4.0.x–4.2.x that
representation is always the marker, so a repeat is always a no-op; on 4.4 a marked registration is
escalated to a deletion rather than no-opped (R7), which is what makes C5's cleanup a plain re-run.
unregistration (≥
4.4-IV2): it writes a realUnregisterControllerRecordinstead of the marker(D8). Not implemented by this patch — that MV does not exist on 4.0.x–4.2.x, so the arm is added
by the 4.4 forward-port (Phase 6.6). It is stated here because it defines the operation, not because
it is in scope.
3.2 Relationship to apache#22191
decommission-controlleris deliberately not namedunregister-controller, because on4.0.x–4.2.x it does not unregister anything: it retires the controller from upgrade decisions and
leaves the registration in place (§ Terminology). What it must not do is invent protocol. On the
wire it is byte-identical to upstream's KIP-1312 RPC — apiKey 94, v0, flexible
0+, a singleControllerIdfield,ALTERonCLUSTER, error code 136 — so a patched 4.0–4.2 client reaching avanilla 4.4+ controller gets precisely the escalated behaviour of R7, and vice versa. Every
identifier we introduce is named
Decommission*(D9):DecommissionControllerRequest.json,ApiKeys.DECOMMISSION_CONTROLLER,DecommissionControllerRequest/Response,DecommissionControllerOptions/Result,KafkaAdminClient#decommissionController,ControllerApis.handleDecommissionController.Unregister*names belong to KAFKA-20395: Support unregistering controllers apache/kafka#22191 andare not introduced by this patch. Compatibility here rests on the apiKey and the schema, which are
what goes over the socket — not on local symbol names.
4.4-IV2a decommissioned controller is amarked registration; at or above it, an absent one. Both mean "does not participate in upgrade
decisions", which is the property R2 needs. Accepted differences on the low-MV representation:
DescribeClusterand MetadataShell still list the controller, andControllerRegistrationManager's "registration removed for this node ID" branch stays dead code.Errors130–135, MVfeature levels ≥ 27, and no fork-local
MetadataVersionvalue. Taking 94 and 136 is not squatting:they are the numbers upstream uses for this exact wire shape (C1).
decommission-controllercommand plus theDecommission*protocol, client and handler artefactslisted in C1. Forbidden: new apiKey or error numbers, new or altered fields on the request schema,
new flags or changed semantics on upstream commands (the KAFKA-20295: Add opt-in workaround for stale controller registrations during metadata.version upgrades #69 contrast, §2.3), additions to the
public
Admininterface.(R7/D8) the operator has nothing to re-run, but the fork's 4.4 must keep the read-side filter
of §4.2 so controllers decommissioned under a low MV stay decommissioned after the upgrade.
Dropping the filter during the forward-port would silently readmit them to upgrade decisions.
Optional cleanup once MV ≥
4.4-IV2is finalized: re-rundecommission-controller, which thenescalates a marked registration into a real deletion (R6 makes the re-run safe, R7 makes it
meaningful). Vanilla, non-fork 4.4 does not honour markers — that is RK2, and it applies to
anyone leaving the fork.
3.3 Safety
MetadataVersion; gate only onisControllerRegistrationSupported()(≥ 3.7-IV0).MetadataRecordSerde.QuorumFeatures.quorumNodeIds()(staticcontroller.quorum.voters), because loop (3b) wouldimmediately re-block upgrades with "controller N has not registered" — trading one blocker for
another. The message must name the config and tell the operator to remove the id from it on every
node first. Deliberate divergence from upstream, which reverted the equivalent check.
3.4 Non-goals
IBP_4_4_IV2or theUnregisterControllerRecordtype. Decommissioning is notunregistration; unregistration stays a 4.4+ capability.
unregister-controller/kafka-metadata-quorum.sh remove-controller --unregistersurfaces, which the fork inheritsuntouched when it reaches 4.4.
Admininterface (D2).4. Design
4.1 Encoding research: a marker with no MV bump (verified 2026-08-03)
Question: can a "decommissioned controller" tag be carried without a metadata change / MV bump,
specifically as a
featureentry?Answer: yes — a feature entry needs no schema change at all and is provably inert. Evidence,
all from unmodified 4.0.x code:
Featuresis an unbounded array in v0, so an extra element is not a schema change: no.jsonedit, no codegen, no record-version bump, no MV bump.RegisterControllerRecord.json:40-48—{"name":"Features","type":"[]ControllerFeature","versions":"0+"}ControllerRegistration.java:75-80ControllerRegistration.java:185-191(toRecord), used byClusterImage.write():85reasonNotSupportedlooks up only the requested name, so an extra key is never consulted.FeatureControlManager.java:278—getOrDefault(featureName, QuorumFeatures.DISABLED)updateFeaturecallsFeature.featureFromName, which throws for unknown names, andreasonNotLocallySupportedrejects it first anyway.FeatureControlManager.java:230-236;Feature.java:205-211kafka-features.sh describe/ApiVersionsResponse— those read the finalized-features image, not registrations.ControllerServer.scala:152-158;FeatureCommand.java:229DescribeCluster— that projection reads onlylisteners().ControllerRegistrationsPublisher.java:63-77processRegistrationFeature-style rejection.ClusterControlManager.java:463-485vs:487-509Featuresfrom the request. R3 is free.ClusterControlManager.java:469-475__decommissioned_controller: 1-1— useful for auditing.ControllerRegistration.toString():216-230;ClusterImageControllersNode.java:57A tagged field would be worse. It is also technically MV-free — old readers park unknown tags in
_unknownTaggedFieldsand re-serialize them (MessageDataGenerator.java:526,:801) — but onlyfor the same message object.
ClusterImage.write()builds a fresh record fromControllerRegistration, whose builder reads only declared fields, so the tag is dropped: anyunpatched node that emits a local snapshot and later restarts loses the marker permanently. It also
costs a schema edit on three branches and risks colliding with a future upstream tag number.
Sharp edges
getOrDefault(featureName, DISABLED)(E4) means a feature name absent from a registration istreated as unsupported, not unknown. A stale 3.7-era registration therefore already blocks
upgrades of any feature it never advertised — relevant to sizing option G.
ControllerFeatureCollection extends ImplicitLinkedHashMultiCollection(
MessageDataGenerator.java:195), soadd()permits duplicate names. The write path must beidempotent.
VersionRange.ofperforms no validation (VersionRange.java:36-42), so any sentinel range isaccepted and only tests will catch a mistake.
4.2 Approach: mark and exclude at one choke point
On a
DecommissionControllerRequest(apiKey 94, wire-identical to upstream's, per C1), the activecontroller appends a
RegisterControllerRecordfor the target id, copying the existing registration (IncarnationId,ZkMigrationReady,EndPoints, realFeatures) and adding one extraFeaturesentry:Name__decommissioned_controllerMinSupportedVersion1MaxSupportedVersion1The registration then stays in place. The only behavioural change is to skip marked registrations in
ClusterControlManager.controllerSupportedFeatures():832. That filter is also what the 4.4forward-port must retain (C5), even though 4.4 writes deletions instead of markers.
That one method is the entire production surface. It is called only from
QuorumController.QuorumClusterFeatureSupportDescriber.controllerSupported()(
QuorumController.java:512-513), consumed only byFeatureControlManager.reasonNotSupported()(
FeatureControlManager.java:268). Every feature andmetadata.versiondecision flows through it,and nothing else does.
Payoff: patched and unpatched nodes hold byte-identical metadata images. The registration is
present everywhere; only the feature-gating read differs. There is no divergence to reason about
and no way for a node to "lose" the marker in a locally generated snapshot (E3).
Interaction with loop (3b). Skipping a marked controller in (3a) means it is never added to
foundControllers, so if its id also appears inquorumFeatures.quorumNodeIds()the next loop failswith "controller N has not registered". S3/D4's write-time hard error is what keeps that
unreachable. Residual edge case to document: adding a marked id back into
controller.quorum.voterslater re-blocks upgrades until that controller actually registers.4.3 Option G — also write permissive feature ranges
In the same record, additionally set permissive ranges for the dead controller:
metadata.versionasVersionRange.of(MetadataVersion.MINIMUM_VERSION.featureLevel(), Short.MAX_VALUE)plus an entry forevery name in
Feature.FEATURES(needed because an absent name reads as unsupported, §4.1).Under D6, "does not participate in the decision" and "always answers yes" are outcome-equivalent, so
this is a real alternative rather than a compromise. Its value is that it works using record
contents alone — no interpretation code — so it also holds on unpatched nodes and on vanilla,
non-fork 4.4. Note that C5 already obliges the fork's 4.4 to keep the filter, so G's remaining job
is narrower than it first appears: unpatched active controllers (RK1) and clusters that leave the fork.
Recommendation: write both in the single record. A supplies honest, auditable semantics and the
log line explaining why a controller was skipped; G is the load-bearing part that makes the outcome
hold on readers that do not understand the marker.
Two limits to state plainly:
blocked on a vanilla, non-fork 4.4 cluster. Converting the marker into a real unregistration once
MV allows (R7, Phase 5.4) remains the real fix.
re-registers truthfully (E9) and D4 forbids marking a static voter. The underlying hazard
(marking a live controller lets the cluster finalize an MV it cannot support) is identical under
A alone; G only extends the consequence to unpatched readers. The runbook precondition "target
process is stopped" (Phase 5.3) guards it.
If reviewers reject writing false ranges, ship A alone and accept RK1 and RK2.
4.4 Fallback
If Phase 0 invalidates the feature-entry encoding, the fallback is a tagged field on
RegisterControllerRecord(taggedVersions: "0+", tag 0), accepting the snapshot-loss andtag-collision downsides in §4.1. If that is also unacceptable, ship #69 instead (D1).
4.5 How decommissioning differs from upstream unregistration
unregister-controller— delete the registrationdecommission-controller— retire the controller from upgrade decisionsUnregisterControllerRecordapiKey 29RegisterControllerRecordapiKey 27IBP_4_4_IV2isControllerRegistrationSupported()(≥ 3.7-IV0)DescribeCluster/ MetadataShellAdminmethodAdmin#unregisterControllerKafkaAdminClient#decommissionControllerUnregisterController*DecommissionController*The last two rows are the crux: identical bytes, different names. Since the request carries nothing
but a controller id and the same authorization, the active controller alone decides what
"decommission" costs — a marker below MV
4.4-IV2, a deletion at or above it (R7/D8) — and a patched4.0–4.2 client keeps interoperating with a 4.4+ controller in both directions, because apiKey 94 v0
decodes the same either way. The names never travel, so they cost nothing at runtime; they cost one
deletion at forward-port time (RK11).
5. Risk register
DescribeClusterand read its presence as failuretoString()(E10)controller.quorum.voters__decommissioned_controller__prefix; unit test asserting the name is absent fromFeature.FEATURESAdmin→KafkaAdminClient(D2)KafkaAdminClientAdmin; tool tests loseMockAdminClientTerseExceptionon cast failure; cover via integration testsdecommission-controllerandunregister-controllerexist--helptext and runbook state the lifecycle (§ Terminology): decommission is the action, unregistration is what it escalates toUnregisterController*schema lands on apiKey 94 alongside ourDecommissionController*.jsonfiles claiming one apiKey — duplicateApiMessageTypeentries, so codegen or compilation fails.json,ApiKeysentry and request/response classes, and repointsKafkaAdminClient#decommissionControllerand the handler at upstream's types. Phase 5.5 records it; Phase 6.6 owns it