Repository navigation
fix(inkless:consolidation): start consolidation when remote storage is enabled later [KC-540] - #802
Merged
Merged
Conversation
viktorsomogyi
force-pushed
the
svv/fix-tsu-start
branch
from
September 16, 2026 15:43
f481d79 to
a18cdf8
Compare
viktorsomogyi
marked this pull request as ready for review
September 17, 2026 07:58
…s enabled later [KC-540] Enabling `remote.storage.enable` on an already-diskless topic co-commits a leader-epoch bump so brokers re-enter become-leader and arm the consolidator. WAL-gap fetches prove a remote prefix from RLMM: present-true returns OFFSET_MOVED, present-false leaves OFFSET_OUT_OF_RANGE, and empty returns NOT_LEADER_OR_FOLLOWER until `__remote_log_metadata` catch-up. The wait is exposed on ConsolidationRemotePrefixUnknown, which resets on re-arm and fetcher removal. ## Test plan - Controller: late remote.enable emits epoch-bump PartitionChangeRecords; skip when consolidation is off, the topic is classic, or the same request starts a switch. - Endpoint: RLMM evidence selects OFFSET_MOVED, OFFSET_OUT_OF_RANGE, or NOT_LEADER_OR_FOLLOWER on a WAL-gap fetch. - Metrics: registerPartition resets the unknown latch; fetcher removal clears it and leaves lag gauges registered. Co-authored-by: Cursor <cursoragent@cursor.com>
viktorsomogyi
force-pushed
the
svv/fix-tsu-start
branch
from
September 17, 2026 11:08
a18cdf8 to
357ccdf
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Transitional remote-segment states can cause invalid tier redirects, and a removal race can leave the new alerting gauge permanently stale.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Enables consolidation when remote storage is activated after a diskless topic’s creation.
Changes:
- Atomically adds leader-epoch bumps to successful late remote-storage configuration changes.
- Detects remote-prefix availability during WAL-gap recovery and exposes an unknown-state metric.
- Updates consolidation lifecycle handling, tests, configuration help, and documentation.
File summaries
| File | Description |
|---|---|
storage/.../RemoteLogManagerTest.java |
Tests remote-segment detection states. |
storage/.../RemoteLogManager.java |
Queries RLMM for remote-prefix evidence. |
server-common/.../ServerConfigs.java |
Documents late remote-storage enablement. |
metadata/.../ReplicationControlManagerInklessTest.java |
Tests epoch-bump generation and exclusions. |
metadata/.../ReplicationControlManager.java |
Generates consolidation epoch-bump records. |
metadata/.../QuorumController.java |
Adds bumps to alter-config batches. |
docs/inkless/GLOSSARY.md |
Updates diskless-topic terminology. |
docs/inkless/FEATURES.md |
Documents supported configuration transitions. |
docs/inkless/DISKLESS_CONSOLIDATION.md |
Documents startup, recovery, and metrics. |
core/.../ReplicaManagerInklessTest.scala |
Tests late consolidation startup. |
core/.../DisklessLeaderEndPointTest.scala |
Tests WAL-gap outcomes. |
core/.../ConsolidationReconcilerTest.scala |
Tests metric registration ordering. |
core/.../ConsolidationFetcherThreadTest.scala |
Tests the remote-prefix gauge lifecycle. |
core/.../ReplicaManager.scala |
Exposes RLMM evidence and metric updates. |
core/.../DisklessLeaderEndPoint.scala |
Selects WAL-gap recovery behavior. |
core/.../ConsolidationReconciler.scala |
Registers metrics before fetchers start. |
core/.../ConsolidationMetrics.scala |
Adds remote-prefix unknown gauges. |
core/.../ConsolidationFetcherManager.scala |
Clears the gauge when removing fetchers. |
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…undary OFFSET_MOVED looks up disklessStartOffset-1, so a seal, a later WAL copy, or any live RLMM segment is not enough to redirect. Prove a COPY_SEGMENT_FINISHED segment covers that offset before signalling the tier-state rebuild. Co-authored-by: Cursor <cursoragent@cursor.com>
… existing image The become-leader test rebuilt the partition from TopicsImage.EMPTY, so localChanges treated it as a new replica and never proved that a same-leader PartitionChangeRecord re-enters applyLocalLeadersDelta. Co-authored-by: Cursor <cursoragent@cursor.com>
leader.fetch() runs outside partitionMapLock, so an in-flight WAL-gap mark could restore ConsolidationRemotePrefixUnknown after fetcher removal. Capture a generation before the RLMM query and bump it on register and removal so a stale mark cannot latch the gauge. Co-authored-by: Cursor <cursoragent@cursor.com>
…elayed fetch removeFetcherForPartitions can bump the generation while awaitDelayedFetch is parked. Overlay marks with the generation captured at fetch start so a stale WAL-gap wait cannot restore ConsolidationRemotePrefixUnknown. Co-authored-by: Cursor <cursoragent@cursor.com>
…across unregister unregisterPartition deleted the generation counter, so re-register reused 1 and an in-flight fetch that captured 1 could restore ConsolidationRemotePrefixUnknown. Bump and keep the counter so stopPartitions and a later re-arm cannot reuse that token. Co-authored-by: Cursor <cursoragent@cursor.com>
| // Capture before awaitDelayedFetch. The delayed op parks up to | ||
| // diskless.consolidation.fetch.max.wait.ms, and removeFetcherForPartitions | ||
| // can bump the generation while this thread is blocked. | ||
| val remotePrefixGenerations = fetchInfos.asScala.keys.iterator.map { tidp => |
Contributor
There was a problem hiding this comment.
This still leaves a race before the generation snapshot. AbstractFetcherThread builds the request while holding partitionMapLock, releases the lock, and only then calls leader.fetch. Removal can happen in that gap and bump the generation. The already-built stale request then enters here, captures the new generation, and its metric update is accepted. Capture the generation while building the request under partitionMapLock and carry it into response processing. Add a test that removes the partition after buildFetch returns but before leader.fetch starts.
Contributor
Author
There was a problem hiding this comment.
Added a fix for this.
…Fetch AbstractFetcherThread releases partitionMapLock between buildFetch and leader.fetch, so removeFetcher can bump the generation in that gap. Snapshot under the lock and carry it into overlay so a stale WAL-gap mark cannot restore ConsolidationRemotePrefixUnknown. Co-authored-by: Cursor <cursoragent@cursor.com>
viktorsomogyi
force-pushed
the
svv/fix-tsu-start
branch
from
September 17, 2026 13:47
aca0f67 to
d5510d9
Compare
…in fetch A missing buildFetch snapshot used to read the generation after partitionMapLock dropped, which is the value overlay must not adopt. Treat a miss as 0 so generationMatches rejects the mark. Co-authored-by: Cursor <cursoragent@cursor.com>
| def unregisterPartition(tp: TopicPartition): Unit = { | ||
| // Bump and keep the generation counter. Deleting it would let re-register reuse 1 | ||
| // and an in-flight fetch that captured 1 could restore the latch after stopPartitions. | ||
| bumpRemotePrefixGeneration(tp) |
Contributor
There was a problem hiding this comment.
Non-blocking: Keeping the counter fixes generation reuse, but it retains the TopicPartition and topic name for every partition until broker shutdown. Repeated topic churn can therefore grow this map without bound. A broker-wide AtomicLong could assign globally unique generation tokens, allowing unregisterPartition to remove the per-partition entry without letting a later registration reuse an old token. This can also be addressed in a follow-up PR.
Contributor
Author
There was a problem hiding this comment.
Given the PR size, I think we should do it as a follow-up.
giuseppelillo
approved these changes
Sep 17, 2026
giuseppelillo
pushed a commit
that referenced
this pull request
Sep 18, 2026
…s enabled later [KC-540] (#802) * fix(inkless:consolidation): start consolidation when remote storage is enabled later [KC-540] Enabling `remote.storage.enable` on an already-diskless topic co-commits a leader-epoch bump so brokers re-enter become-leader and arm the consolidator. WAL-gap fetches prove a remote prefix from RLMM: present-true returns OFFSET_MOVED, present-false leaves OFFSET_OUT_OF_RANGE, and empty returns NOT_LEADER_OR_FOLLOWER until `__remote_log_metadata` catch-up. The wait is exposed on ConsolidationRemotePrefixUnknown, which resets on re-arm and fetcher removal. ## Test plan - Controller: late remote.enable emits epoch-bump PartitionChangeRecords; skip when consolidation is off, the topic is classic, or the same request starts a switch. - Endpoint: RLMM evidence selects OFFSET_MOVED, OFFSET_OUT_OF_RANGE, or NOT_LEADER_OR_FOLLOWER on a WAL-gap fetch. - Metrics: registerPartition resets the unknown latch; fetcher removal clears it and leaves lag gauges registered. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): require remote coverage of the WAL-gap boundary OFFSET_MOVED looks up disklessStartOffset-1, so a seal, a later WAL copy, or any live RLMM segment is not enough to redirect. Prove a COPY_SEGMENT_FINISHED segment covers that offset before signalling the tier-state rebuild. Co-authored-by: Cursor <cursoragent@cursor.com> * test(inkless:consolidation): replay the late-enable epoch bump on the existing image The become-leader test rebuilt the partition from TopicsImage.EMPTY, so localChanges treated it as a new replica and never proved that a same-leader PartitionChangeRecord re-enters applyLocalLeadersDelta. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): ignore stale remote-prefix unknown updates leader.fetch() runs outside partitionMapLock, so an in-flight WAL-gap mark could restore ConsolidationRemotePrefixUnknown after fetcher removal. Capture a generation before the RLMM query and bump it on register and removal so a stale mark cannot latch the gauge. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): capture remote-prefix generation before delayed fetch removeFetcherForPartitions can bump the generation while awaitDelayedFetch is parked. Overlay marks with the generation captured at fetch start so a stale WAL-gap wait cannot restore ConsolidationRemotePrefixUnknown. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): keep remote-prefix generations monotonic across unregister unregisterPartition deleted the generation counter, so re-register reused 1 and an in-flight fetch that captured 1 could restore ConsolidationRemotePrefixUnknown. Bump and keep the counter so stopPartitions and a later re-arm cannot reuse that token. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): capture remote-prefix generation in buildFetch AbstractFetcherThread releases partitionMapLock between buildFetch and leader.fetch, so removeFetcher can bump the generation in that gap. Snapshot under the lock and carry it into overlay so a stale WAL-gap mark cannot restore ConsolidationRemotePrefixUnknown. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): drop live remote-prefix generation reads in fetch A missing buildFetch snapshot used to read the generation after partitionMapLock dropped, which is the value overlay must not adopt. Treat a miss as 0 so generationMatches rejects the mark. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
giuseppelillo
pushed a commit
that referenced
this pull request
Sep 18, 2026
…s enabled later [KC-540] (#802) * fix(inkless:consolidation): start consolidation when remote storage is enabled later [KC-540] Enabling `remote.storage.enable` on an already-diskless topic co-commits a leader-epoch bump so brokers re-enter become-leader and arm the consolidator. WAL-gap fetches prove a remote prefix from RLMM: present-true returns OFFSET_MOVED, present-false leaves OFFSET_OUT_OF_RANGE, and empty returns NOT_LEADER_OR_FOLLOWER until `__remote_log_metadata` catch-up. The wait is exposed on ConsolidationRemotePrefixUnknown, which resets on re-arm and fetcher removal. - Controller: late remote.enable emits epoch-bump PartitionChangeRecords; skip when consolidation is off, the topic is classic, or the same request starts a switch. - Endpoint: RLMM evidence selects OFFSET_MOVED, OFFSET_OUT_OF_RANGE, or NOT_LEADER_OR_FOLLOWER on a WAL-gap fetch. - Metrics: registerPartition resets the unknown latch; fetcher removal clears it and leaves lag gauges registered. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): require remote coverage of the WAL-gap boundary OFFSET_MOVED looks up disklessStartOffset-1, so a seal, a later WAL copy, or any live RLMM segment is not enough to redirect. Prove a COPY_SEGMENT_FINISHED segment covers that offset before signalling the tier-state rebuild. Co-authored-by: Cursor <cursoragent@cursor.com> * test(inkless:consolidation): replay the late-enable epoch bump on the existing image The become-leader test rebuilt the partition from TopicsImage.EMPTY, so localChanges treated it as a new replica and never proved that a same-leader PartitionChangeRecord re-enters applyLocalLeadersDelta. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): ignore stale remote-prefix unknown updates leader.fetch() runs outside partitionMapLock, so an in-flight WAL-gap mark could restore ConsolidationRemotePrefixUnknown after fetcher removal. Capture a generation before the RLMM query and bump it on register and removal so a stale mark cannot latch the gauge. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): capture remote-prefix generation before delayed fetch removeFetcherForPartitions can bump the generation while awaitDelayedFetch is parked. Overlay marks with the generation captured at fetch start so a stale WAL-gap wait cannot restore ConsolidationRemotePrefixUnknown. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): keep remote-prefix generations monotonic across unregister unregisterPartition deleted the generation counter, so re-register reused 1 and an in-flight fetch that captured 1 could restore ConsolidationRemotePrefixUnknown. Bump and keep the counter so stopPartitions and a later re-arm cannot reuse that token. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): capture remote-prefix generation in buildFetch AbstractFetcherThread releases partitionMapLock between buildFetch and leader.fetch, so removeFetcher can bump the generation in that gap. Snapshot under the lock and carry it into overlay so a stale WAL-gap mark cannot restore ConsolidationRemotePrefixUnknown. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): drop live remote-prefix generation reads in fetch A missing buildFetch snapshot used to read the generation after partitionMapLock dropped, which is the value overlay must not adopt. Treat a miss as 0 so generationMatches rejects the mark. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
giuseppelillo
pushed a commit
that referenced
this pull request
Sep 24, 2026
…s enabled later [KC-540] (#802) * fix(inkless:consolidation): start consolidation when remote storage is enabled later [KC-540] Enabling `remote.storage.enable` on an already-diskless topic co-commits a leader-epoch bump so brokers re-enter become-leader and arm the consolidator. WAL-gap fetches prove a remote prefix from RLMM: present-true returns OFFSET_MOVED, present-false leaves OFFSET_OUT_OF_RANGE, and empty returns NOT_LEADER_OR_FOLLOWER until `__remote_log_metadata` catch-up. The wait is exposed on ConsolidationRemotePrefixUnknown, which resets on re-arm and fetcher removal. ## Test plan - Controller: late remote.enable emits epoch-bump PartitionChangeRecords; skip when consolidation is off, the topic is classic, or the same request starts a switch. - Endpoint: RLMM evidence selects OFFSET_MOVED, OFFSET_OUT_OF_RANGE, or NOT_LEADER_OR_FOLLOWER on a WAL-gap fetch. - Metrics: registerPartition resets the unknown latch; fetcher removal clears it and leaves lag gauges registered. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): require remote coverage of the WAL-gap boundary OFFSET_MOVED looks up disklessStartOffset-1, so a seal, a later WAL copy, or any live RLMM segment is not enough to redirect. Prove a COPY_SEGMENT_FINISHED segment covers that offset before signalling the tier-state rebuild. Co-authored-by: Cursor <cursoragent@cursor.com> * test(inkless:consolidation): replay the late-enable epoch bump on the existing image The become-leader test rebuilt the partition from TopicsImage.EMPTY, so localChanges treated it as a new replica and never proved that a same-leader PartitionChangeRecord re-enters applyLocalLeadersDelta. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): ignore stale remote-prefix unknown updates leader.fetch() runs outside partitionMapLock, so an in-flight WAL-gap mark could restore ConsolidationRemotePrefixUnknown after fetcher removal. Capture a generation before the RLMM query and bump it on register and removal so a stale mark cannot latch the gauge. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): capture remote-prefix generation before delayed fetch removeFetcherForPartitions can bump the generation while awaitDelayedFetch is parked. Overlay marks with the generation captured at fetch start so a stale WAL-gap wait cannot restore ConsolidationRemotePrefixUnknown. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): keep remote-prefix generations monotonic across unregister unregisterPartition deleted the generation counter, so re-register reused 1 and an in-flight fetch that captured 1 could restore ConsolidationRemotePrefixUnknown. Bump and keep the counter so stopPartitions and a later re-arm cannot reuse that token. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): capture remote-prefix generation in buildFetch AbstractFetcherThread releases partitionMapLock between buildFetch and leader.fetch, so removeFetcher can bump the generation in that gap. Snapshot under the lock and carry it into overlay so a stale WAL-gap mark cannot restore ConsolidationRemotePrefixUnknown. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(inkless:consolidation): drop live remote-prefix generation reads in fetch A missing buildFetch snapshot used to read the generation after partitionMapLock dropped, which is the value overlay must not adopt. Treat a miss as 0 so generationMatches rejects the mark. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Summary
Enabling
remote.storage.enableon an already-diskless topic is a config-only write today, so brokers never re-enter become-leader and consolidation never arms. When cluster consolidation is on, the controller co-commits a leader-epoch bump with thatConfigRecord. Brokers re-enter become-leader, create the local log if needed, and start the consolidation fetcher. The bump is skipped when consolidation is off, the topic is not already diskless, or the same request starts a classic-to-diskless switch.A WAL-gap fetch after that arm can land below the diskless WAL start while RLMM is still catching up. The endpoint proves a remote prefix from RLMM (or a cheap local positive): present-true returns
OFFSET_MOVED_TO_TIERED_STORAGE, present-false leaves never-tieredOFFSET_OUT_OF_RANGE, and empty returnsNOT_LEADER_OR_FOLLOWERso the fetcher retries at debug.ConsolidationRemotePrefixUnknownis 1 during that wait and 0 once decided. Register resets the latch; fetcher removal clears it without tearing down the lag gauges. Alert on a sustained non-zero after__remote_log_metadatacatch-up; a leadership transition or an overlapping fetch can dip or briefly restore the value.Test plan
remote.storage.enable=trueon an already-diskless topic emits epoch-bumpPartitionChangeRecordobjects in the same batch as theConfigRecord.OFFSET_MOVED; present-false →OFFSET_OUT_OF_RANGE; empty →NOT_LEADER_OR_FOLLOWER.registerPartitionresetsConsolidationRemotePrefixUnknown; fetcher removal clears the latch and leaves lag gauges registered.