Repository navigation
docs(inkless:switch): describe the last-known-leader election in phase 1 [KC-759] - #871
Conversation
…e 1 [KC-759] The phase 1 guards block unclean leader election, but they don't block the last-known-leader election that KIP-966 adds. With ELR enabled, a partition whose ISR and ELR members all shut down uncleanly elects its last known leader when that broker returns, resets the ISR to it, and loses any records that broker lost. The docs said such a partition has no leader until an ISR member returns. The switch doc now describes when the controller runs this election and what it does to a partition in phase 1. If the leader lost records below the seal, the leader marks the partition offline, the controller keeps it as leader, and phase 2 doesn't start. The doc gives the recovery: a downward re-seal to the leader's log end offset, a leader restart, and a control-plane repair. That recovery loses the records between the leader's log end offset and the old seal. Testing `ClassicToDisklessSwitchLastKnownLeaderClusterTest` holds phase 1 open with a gated remote storage manager, drives the partition into the last-known-leader election through unclean shutdowns, and checks that the leader marks the partition offline. It then runs the re-seal recovery and checks that phase 2 starts and that a consumer reads the retained classic records followed by the diskless records. Assisted-by: Claude Opus 5.5
…ng them The last-known-leader test logged each stage while it was being written. It now asserts those states: the leader recovery state after the election, the seal command's offset range, the partition staying offline after the re-seal until the leader restarts, and the log end offset on every replica in phase 2. The offline check waits for the leader to apply the lower seal instead of sleeping for a fixed time. Assisted-by: Claude Opus 5.5
There was a problem hiding this comment.
🟡 Changes recommended
The recovery test can fail nondeterministically because it checks replica log end offsets after opening the upload gate, allowing phase-2 fetchers to advance them before the assertion runs.
1 open issue
What changed in this PR
This PR documents how KIP-966 last-known-leader elections affect phase 1 of Inkless’s classic-to-diskless switch, without changing controller behavior.
Changes:
- Clarifies that phase 1 guards don't block last-known-leader elections.
- Documents data-loss consequences and recovery through lowering the seal, restarting the leader, and repairing the control plane.
- Adds a cluster test covering the election, offline partition, recovery, and consumption across missing offsets.
| File | Description |
|---|---|
docs/inkless/CLASSIC_TO_DISKLESS_SWITCH.md |
Explains election behavior, failure outcomes, and recovery steps. |
core/src/test/java/kafka/server/ClassicToDisklessSwitchLastKnownLeaderClusterTest.java |
Tests last-known-leader data loss and recovery with gated remote uploads. |
Review effort: Balanced
Once the upload gate opens, phase 2 consolidates from the seal, and a replica can append the diskless records and move its log end offset past the seal before the test checks it. The test now checks the log end offsets after the ISR recovers and before it opens the gate, and asserts that phase 2 hasn't started at that point. Assisted-by: Claude Opus 5.5
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified in the documentation and cluster-test changes covering last-known-leader recovery during classic-to-diskless switching.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
|
|
||
| ### Last-known-leader election | ||
|
|
||
| If you enable the `eligible.leader.replicas.version` feature, the controller tracks eligible leader replicas (ELR) as described in KIP-966. KIP-966 adds a last-known-leader election, and the phase 1 guards don't block it. The controller runs this election for every election type, not only `UNCLEAN`, and `unclean.leader.election.enable` doesn't control it. |
There was a problem hiding this comment.
Seems that ELRV_1 is LATEST_PRODUCTION (EligibleLeaderReplicasVersion.java:27,31), so a 4.1 or later cluster has ELR on without anyone enabling it. "If you enable" reads as an opt-in. Can you reword it to say it's the default?
| If you enable the `eligible.leader.replicas.version` feature, the controller tracks eligible leader replicas (ELR) as described in KIP-966. KIP-966 adds a last-known-leader election, and the phase 1 guards don't block it. The controller runs this election for every election type, not only `UNCLEAN`, and `unclean.leader.election.enable` doesn't control it. | |
| The `eligible.leader.replicas.version` feature is on by default at 4.1 and later. With it on, the controller tracks eligible leader replicas (ELR) as described in KIP-966. KIP-966 adds a last-known-leader election, and the phase 1 guards don't block it. The controller runs this election for every election type, not only `UNCLEAN`, and `unclean.leader.election.enable` doesn't control it. |
There was a problem hiding this comment.
Applied in bf9a65e. The default comes from Feature.defaultLevel, so a cluster formatted at 4.1 or later has ELR on, and so does one upgraded with kafka-features.sh upgrade --release-version to 4.1 or later. An upgrade that sets only --metadata leaves the feature at its old level.
| return registration(0, topic).leader; | ||
| } | ||
|
|
||
| private void driveIntoLastKnownLeaderBelowSeal(final Admin admin, final String topic, final int leaderHint) throws Exception { |
There was a problem hiding this comment.
The doc describes two outcomes, and the test runs the one where the leader lost records. I added a second case that skips the truncation. It passes: the leader stays online, the high watermark is 20, the ISR returns to 3, the controller records RECOVERED, phase 2 starts after the gate, and the consumer reads 0 to 24. The high watermark check doesn't prove the reconcile code, because makeLeader advances it with an ISR of one.
This can be parametrized:
private void driveIntoLastKnownLeader(final Admin admin, final String topic, final int leaderHint,
final boolean loseTail) throws Exception {
...
if (loseTail) {
final UnifiedLog leaderLog = cluster.brokers().get(leader).replicaManager().getLog(tp).get();
leaderLog.truncateTo(TRUNCATED_LEO);
assertEquals(TRUNCATED_LEO, leaderLog.logEndOffset());
}
...
if (loseTail) {
waitForCondition(() -> isOffline(leader, tp), "the leader to mark the partition offline");
}
}
and a new test pass the flags to pin the behavior to both outcomes.
There was a problem hiding this comment.
Done in bf9a65e: the test is parametrized on loseTail. One difference from the snippet: the ISR-of-one and RECOVERING assertions also sit under loseTail. Without the loss, the leader can clear both through AlterPartition before the poll reads the registration. The loseTail=false case waits for RECOVERED instead, then checks that the ISR returns to 3, every log ends at 20 in phase 1, phase 2 starts, and the consumer reads 0 to 24. Both cases passed in two runs.
…ection The switch doc describes two outcomes of the last-known-leader election in phase 1, and the test ran only the one where the leader lost records. The test is now parametrized on that loss. Without it, the leader keeps the partition online and finishes recovery, the ISR returns to 3, phase 2 starts at the original seal, and a consumer reads every record. The doc also says that the `eligible.leader.replicas.version` feature is on by default at 4.1 and later, instead of reading as an opt-in. Assisted-by: Claude Opus 5.5
With eligible leader replicas (ELR) enabled, a partition in phase 1 of the classic-to-diskless switch can still elect a leader from outside its ISR.
CLASSIC_TO_DISKLESS_SWITCH.mdsays that a partition that loses every ISR member has no leader until one of them returns. The last-known-leader election of KIP-966 contradicts that, and the docs don't describe it.Problem
The phase 1 guards in
ReplicationControlManagerblock unclean leader election by not callingsetElection(Election.UNCLEAN).PartitionChangeBuilder.electAnyLeaderandelectPreferredLeadercallcanElectLastKnownLeader()before they check the election type. So the last-known-leader election runs for every election type, andunclean.leader.election.enabledoesn't control it.The following example uses replicas
[2, 0, 1],min.insync.replicas=2, and a seal at 20:lastKnownElr[2, 0, 1][][][2][0][][][0, 2][2][][2][2][2][][]RECOVERINGThe partition loses offsets 15 to 19, even though the high watermark covered them. KIP-966 accepts the same loss for a classic topic. In phase 1, the loss shows up differently:
Change
The switch doc gains a "Last-known-leader election" subsection. It describes when the controller runs the election, what happens to a partition in phase 1, and how to recover one that's offline:
kafka-topic-switch.sh seal.kafka-topic-switch.sh repair.The recovery loses the records between the leader's log end offset and the old seal. The "Unclean leader election" section and the phase table now point to the new subsection.
This change doesn't modify controller behavior. Blocking the election in phase 1 would leave the partition without a leader in both outcomes: the one where the last known leader holds the whole prefix, and the one where it doesn't.
Testing
ClassicToDisklessSwitchLastKnownLeaderClusterTestruns the sequence in the example on a 3-broker cluster with ELR enabled. A gated remote storage manager fails segment copies until the test releases it. Without the gate, the leader rolls its active segment at the seal and the partition leaves phase 1 within one task interval. The test deletes the clean-shutdown markers, so the controller sees unclean shutdowns.The test is parametrized on
loseTail, so it covers both outcomes in the doc.With
loseTail=true, the test truncates the leader's log to 15 before the leader stops. It then checks the following:RECOVERINGstate, and the leader marks the partition offline.kafka-topic-switch.sh sealvalidates against. After the seal is lowered, the partition stays offline until the leader restarts.auto.offset.reset=nonereads offsets 0 to 14 and the diskless records at 20 to 24.With
loseTail=false, the test checks the following:The test doesn't run the recovery without the repair step, or the election while the switch is pending. The doc describes the pending case from the code.
KC-759
🤖 Generated with Claude Code
Assisted-by: Claude Opus 5.5