Skip to content

fix(inkless:consolidation): don't fence a consolidating leader below the seal#677

Merged
viktorsomogyi merged 2 commits into
mainfrom
svv/ts-unification-leader-below-seal
Jul 1, 2026
Merged

fix(inkless:consolidation): don't fence a consolidating leader below the seal#677
viktorsomogyi merged 2 commits into
mainfrom
svv/ts-unification-leader-below-seal

Conversation

@viktorsomogyi

Copy link
Copy Markdown
Contributor

maybeReconcileSwitchedLeader fenced a switched leader offline whenever its LEO fell below the committed seal. It runs during applyLocalLeadersDelta, before the ConsolidationReconciler, so the wiped-leader recovery from #673 never ran. The reconciler only sees online partitions, and a wiped consolidating leader (LEO 0 < seal) was offline before it could rebuild. Every fetch and offset request then failed with KAFKA_STORAGE_ERROR.

For a consolidating diskless topic with remote storage enabled, [0, seal) is in the remote tier and can be rebuilt, so leave the partition online and let the reconciler arm consolidation at the current LEO. Non-consolidating topics still fence, since their classic prefix has no remote copy and LEO < seal is unrecoverable corruption.

Co-authored-by: Cursor cursoragent@cursor.com

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents a switched diskless leader (classic-to-diskless seal committed) from being fenced offline when LEO < seal if the topic can rebuild its classic prefix from remote storage (consolidating diskless with remote enabled). This ensures the ConsolidationReconciler can arm consolidation and recover a wiped leader instead of leaving the partition offline and causing KAFKA_STORAGE_ERROR on reads.

Changes:

  • Update ReplicaManager.maybeReconcileSwitchedLeader to avoid fencing leaders below the seal when remote recovery is possible.
  • Extend the reconciliation docstring to document the new recovery path and why fencing blocks it.
  • Add a unit test covering the “consolidating leader below seal with remote enabled stays online” case and adjust the test harness to correctly enable UnifiedLog.remoteLogEnabled().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
core/src/main/scala/kafka/server/ReplicaManager.scala Changes switched-leader reconciliation behavior for LEO < seal to preserve availability for remote rebuild.
core/src/test/scala/unit/kafka/server/ReplicaManagerInklessTest.scala Adds coverage for the new recovery behavior and fixes test setup so remote log is actually enabled in UnifiedLog.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/src/main/scala/kafka/server/ReplicaManager.scala

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread core/src/test/scala/unit/kafka/server/ReplicaManagerInklessTest.scala Outdated
@viktorsomogyi viktorsomogyi force-pushed the svv/ts-unification-leader-below-seal branch from 7faa8c0 to 53c392e Compare July 1, 2026 09:06
…the seal

maybeReconcileSwitchedLeader fenced a switched leader offline whenever its LEO
fell below the committed seal. It runs during applyLocalLeadersDelta, before the
ConsolidationReconciler, so the wiped-leader recovery from #673 never ran. The
reconciler only sees online partitions, and a wiped consolidating leader
(LEO 0 < seal) was offline before it could rebuild. Every fetch and offset
request then failed with KAFKA_STORAGE_ERROR.

For a consolidating diskless topic with remote storage enabled, [0, seal) is in
the remote tier and can be rebuilt, so leave the partition online and let the
reconciler arm consolidation at the current LEO. Non-consolidating topics still
fence, since their classic prefix has no remote copy and LEO < seal is
unrecoverable corruption.

Co-authored-by: Cursor <cursoragent@cursor.com>
@viktorsomogyi viktorsomogyi force-pushed the svv/ts-unification-leader-below-seal branch from 53c392e to e526d64 Compare July 1, 2026 09:08
@viktorsomogyi viktorsomogyi marked this pull request as ready for review July 1, 2026 09:10
@viktorsomogyi viktorsomogyi requested a review from jeqo July 1, 2026 09:10

@jeqo jeqo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT, just a small logging improvement

s"classic-to-diskless start offset $seal; cannot catch up from another replica. " +
s"Marking the partition offline as its local log is corrupt below the committed seal.")
markPartitionOffline(tp)
if (isConsolidatingPartition(partition) && log.remoteLogEnabled()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personal note: this can be simplified when #678 lands -- not a blocker

Comment thread core/src/main/scala/kafka/server/ReplicaManager.scala Outdated
@viktorsomogyi viktorsomogyi merged commit 10bc14a into main Jul 1, 2026
5 checks passed
@viktorsomogyi viktorsomogyi deleted the svv/ts-unification-leader-below-seal branch July 1, 2026 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants