fix(inkless:consolidation): DeleteRecords across tiers on consolidating topics#707
Draft
viktorsomogyi wants to merge 1 commit into
Draft
fix(inkless:consolidation): DeleteRecords across tiers on consolidating topics#707viktorsomogyi wants to merge 1 commit into
viktorsomogyi wants to merge 1 commit into
Conversation
…ng topics
Makes `DeleteRecords` correct end-to-end for **switched consolidating** diskless
topics (both `diskless.enable` and `remote.storage.enable`), where the client-facing
leader is a hash/AZ-selected replica (via the metadata transformer) rather than the
real KRaft leader, and a freshly-rebuilt leader's local log start is pinned at the
classic-to-diskless seal.
Changes, in layers:
- **Routing (local leg → real leader).** `DisklessDeleteRecordsForwarder` forwards the
local leg of a `DeleteRecords` request to the actual leader, so the admin request no
longer loops on `NOT_LEADER_OR_FOLLOWER` against a transformer-advertised follower.
Wired through `KafkaApis`/`KafkaApisBuilder`/`BrokerServer`. `DeleteRecordsCommand`
now exits non-zero on per-partition errors.
- **Cross-tier low watermark.** For consolidating partitions the completion low
watermark and `ListOffsets(EARLIEST)` come from the authoritative control-plane
cross-tier earliest, not the ISR (whose follower log starts are frozen at the switch).
`DisklessFetchOffsetRouter` routes `EARLIEST` to the control plane (**Problem A**:
cross-broker earliest consistency), and `DisklessLeaderEndPoint` reports
`min(crossTierEarliest, localLogStart)` as the whole-log start.
- **Reclaim floor (Problem B).** `RemoteLogManager` takes a `logStartOffsetOverride`
(control-plane cross-tier earliest) so a rebuilt leader no longer reclaims the entire
remote classic prefix `[earliest, seal)`.
- **Fail-safe reclaim floor.** When the override is unavailable (control plane down /
metadata not yet propagated), the RLM no longer falls back to the local seal for a
*consolidating* partition — it uses the remote earliest and defers, so a transient
control-plane outage can never trigger irreversible over-deletion. A new
`isConsolidatingDisklessPartition` predicate distinguishes this from classic topics,
which keep the upstream local-log-start floor.
Docs: `DISKLESS_CONSOLIDATION.md` updated; full design/rationale and an
uncertainties/risk-tracking checklist in `DELETE_RECORDS_DISKLESS_ROUTING_IMPL.md`.
- [x] `RemoteLogManagerTest` — reclaim floor uses the override; fail-safe skips reclaim
when the override is absent for consolidating; classic topics unchanged.
- [x] `ReplicaManagerInklessTest` — `crossTierEarliestOffset` branches (cache hit,
miss→CP, CP error/throw→empty, non-consolidating→empty); `isConsolidatingDisklessPartition`.
- [x] `DisklessLeaderEndPointTest` — whole-log start = cross-tier earliest; reverts to
local seal when unavailable.
- [x] `DisklessFetchOffsetRouterTest`, `DisklessDeleteRecordsForwarderTest`, `DeleteRecordsCommandTest`.
- [x] e2e `consolidation_delete_records_across_tiers_test.py` — earliest advances off 0,
is identical on every broker, settles exactly at the delete boundary, and the
surviving prefix reads back contiguous.
- [x] `core`/`storage` compile + checkstyle + spotbugs green.
- [ ] Open items tracked in the checklist in `DELETE_RECORDS_DISKLESS_ROUTING_IMPL.md`
(multi-partition/rapid-failover e2e, retention×DeleteRecords, sync-DB/batching, etc.).
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.
Makes
DeleteRecordscorrect end-to-end for switched consolidating diskless topics (bothdiskless.enableandremote.storage.enable), where the client-facing leader is a hash/AZ-selected replica (via the metadata transformer) rather than the real KRaft leader, and a freshly-rebuilt leader's local log start is pinned at the classic-to-diskless seal.Changes, in layers:
DisklessDeleteRecordsForwarderforwards the local leg of aDeleteRecordsrequest to the actual leader, so the admin request no longer loops onNOT_LEADER_OR_FOLLOWERagainst a transformer-advertised follower. Wired throughKafkaApis/KafkaApisBuilder/BrokerServer.DeleteRecordsCommandnow exits non-zero on per-partition errors.ListOffsets(EARLIEST)come from the authoritative control-plane cross-tier earliest, not the ISR (whose follower log starts are frozen at the switch).DisklessFetchOffsetRouterroutesEARLIESTto the control plane (Problem A: cross-broker earliest consistency), andDisklessLeaderEndPointreportsmin(crossTierEarliest, localLogStart)as the whole-log start.RemoteLogManagertakes alogStartOffsetOverride(control-plane cross-tier earliest) so a rebuilt leader no longer reclaims the entire remote classic prefix[earliest, seal).isConsolidatingDisklessPartitionpredicate distinguishes this from classic topics, which keep the upstream local-log-start floor.Docs:
DISKLESS_CONSOLIDATION.mdupdated; full design/rationale and an uncertainties/risk-tracking checklist inDELETE_RECORDS_DISKLESS_ROUTING_IMPL.md.RemoteLogManagerTest— reclaim floor uses the override; fail-safe skips reclaim when the override is absent for consolidating; classic topics unchanged.ReplicaManagerInklessTest—crossTierEarliestOffsetbranches (cache hit, miss→CP, CP error/throw→empty, non-consolidating→empty);isConsolidatingDisklessPartition.DisklessLeaderEndPointTest— whole-log start = cross-tier earliest; reverts to local seal when unavailable.DisklessFetchOffsetRouterTest,DisklessDeleteRecordsForwarderTest,DeleteRecordsCommandTest.consolidation_delete_records_across_tiers_test.py— earliest advances off 0, is identical on every broker, settles exactly at the delete boundary, and the surviving prefix reads back contiguous.core/storagecompile + checkstyle + spotbugs green.DELETE_RECORDS_DISKLESS_ROUTING_IMPL.md(multi-partition/rapid-failover e2e, retention×DeleteRecords, sync-DB/batching, etc.).Delete this text and replace it with a detailed description of your change. The
PR title and body will become the squashed commit message.
If you would like to tag individuals, add some commentary, upload images, or
include other supplemental information that should not be part of the eventual
commit message, please use a separate comment.
If applicable, please include a summary of the testing strategy (including
rationale) for the proposed change. Unit and/or integration tests are expected
for any behavior change and system tests should be considered for larger
changes.