Repository navigation
fix(inkless:consolidation): freeze cross-tier start during prune - #806
Conversation
af43289 to
6c351c4
Compare
6c351c4 to
f9811c3
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Transitional metadata can still cause an unavailable remote offset to be permanently reported.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Defers remote-start bootstrap for consolidating diskless partitions until a remote segment exists.
Changes:
- Skip reporting when no remote-start override or remote segment exists.
- Add regression coverage for deferred and eventual reporting.
| File | Description |
|---|---|
storage/src/test/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerTest.java |
Tests deferred bootstrap and remote-segment reporting. |
storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManager.java |
Adds the remote-segment bootstrap guard. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
f9811c3 to
f631e6b
Compare
c950bc1 to
0ecc436
Compare
9b88d8d to
222f230
Compare
a3afdf3 to
7ec2cc4
Compare
When consolidation starts after pure-diskless retention, no stored cross-tier start exists. Inferring one from broker-local or remote metadata can publish an unreadable offset or over-delete a switched prefix. Initialize the cross-tier start from the existing WAL start in the same control-plane transaction as the first consolidated prune. Until that handoff completes, RemoteLogManager keeps copying while deferring publication and remote expiration. Allow switched partitions to trigger the handoff when remote storage reaches seal - 1. Co-authored-by: Cursor <cursoragent@cursor.com>
efcf85e to
71bd7db
Compare
There was a problem hiding this comment.
I think the overall approach and design is good. Had it reviewed from the technical perspective with Opus 5.5 and Grok 4.7 and found only a minor test issue. If you could add that extra test, I think we're good to merge.
| } | ||
|
|
||
| @Nested | ||
| class PruneCrossTierBootstrap { |
There was a problem hiding this comment.
AI comment:
The switched handoff needs a test in AbstractControlPlaneTest, so it runs against Postgres and the in-memory control plane. V29 is the production path.
The pruner test already checks that a switched partition submits a prune at seal - 1. Nothing checks what that prune stores. Build the row the way init_diskless_log_v1 does: classic log start below the seal, high watermark and diskless start equal to the seal. Prune at seal - 1 for two shapes, since they take different branches:
- A batch whose base offset is the seal.
- An empty WAL. This reaches the seal only when the high watermark is the seal.
In both cases, remote_log_start_offset stays at the old WAL start, and log_start_offset and EARLIEST_LOCAL move to the seal.
There was a problem hiding this comment.
Added in 289bb19. The shared control-plane contract now initializes a switched partition with a classic start below the seal and covers both requested shapes: a remaining batch based at the seal, and an empty WAL with its high watermark at the seal. Both PostgreSQL and memory verify that the raw cross-tier start and EARLIEST remain at the classic start while log_start_offset and EARLIEST_LOCAL advance to the seal. JAVA_HOME=$(/usr/libexec/java_home -v 17) ./gradlew :storage:inkless:test --tests '*SwitchedPruneCrossTierBootstrap*' --no-daemon --no-parallel passes all four cases.
Exercise the seal - 1 handoff through the shared control-plane contract with both a batch at the seal and an empty WAL. Verify that PostgreSQL and memory preserve the classic cross-tier start while advancing the local start to the seal. Co-authored-by: Cursor <cursoragent@cursor.com>
When consolidation starts after pure-diskless retention, no stored cross-tier start exists. Inferring one from broker-local or remote metadata can publish an unreadable offset or over-delete a switched prefix. The first consolidated prune now freezes the current WAL start as remote_log_start_offset in the same control-plane transaction, then advances log_start_offset. Until that handoff completes, RemoteLogManager keeps copying while deferring its become-leader report and remote expiration. Switched partitions can submit seal - 1 so the handoff can run even when that request removes no WAL records. This prevention-only change does not repair rows that already contain an incorrect remote_log_start_offset. Co-authored-by: Cursor <cursoragent@cursor.com>
When consolidation starts after pure-diskless retention, no stored cross-tier start exists. Inferring one from broker-local or remote metadata can publish an unreadable offset or over-delete a switched prefix. The first consolidated prune now freezes the current WAL start as remote_log_start_offset in the same control-plane transaction, then advances log_start_offset. Until that handoff completes, RemoteLogManager keeps copying while deferring its become-leader report and remote expiration. Switched partitions can submit seal - 1 so the handoff can run even when that request removes no WAL records. This prevention-only change does not repair rows that already contain an incorrect remote_log_start_offset. Co-authored-by: Cursor <cursoragent@cursor.com>
When consolidation starts after pure-diskless retention, no stored cross-tier start exists. Inferring one from broker-local or remote metadata can publish an unreadable offset or over-delete a switched prefix. The first consolidated prune now freezes the current WAL start as remote_log_start_offset in the same control-plane transaction, then advances log_start_offset. Until that handoff completes, RemoteLogManager keeps copying while deferring its become-leader report and remote expiration. Switched partitions can submit seal - 1 so the handoff can run even when that request removes no WAL records. This prevention-only change does not repair rows that already contain an incorrect remote_log_start_offset. Co-authored-by: Cursor <cursoragent@cursor.com>
When consolidation starts after pure-diskless retention, no stored cross-tier start exists. Inferring one from broker-local or remote metadata can publish an unreadable offset or over-delete a switched prefix.
The first consolidated prune now freezes the current WAL start as
remote_log_start_offsetin the same control-plane transaction, then advanceslog_start_offset. Until that handoff completes,RemoteLogManagerkeeps copying while deferring its become-leader report and remote expiration. Switched partitions can submitseal - 1so the handoff can run even when that request removes no WAL records.This prevention-only change does not repair rows that already contain an incorrect
remote_log_start_offset.Test plan
JAVA_HOME=$(/usr/libexec/java_home -v 25) ./gradlew :storage:inkless:generateJooqClasses --rerun-tasks --no-daemon --no-parallelJAVA_HOME=$(/usr/libexec/java_home -v 17) ./gradlew :storage:test --tests org.apache.kafka.server.log.remote.storage.RemoteLogManagerTest --no-daemon --no-parallelJAVA_HOME=$(/usr/libexec/java_home -v 17) ./gradlew :storage:inkless:test --tests '*PruneCrossTierBootstrap*' :core:test --tests io.aiven.inkless.consolidation.ConsolidatedDisklessLogPrunerTest --no-daemon --no-parallelJAVA_HOME=$(/usr/libexec/java_home -v 17) ./gradlew :storage:inkless:test --tests io.aiven.inkless.control_plane.InMemoryControlPlanePruneDisklessLogsTest --tests io.aiven.inkless.control_plane.postgres.PruneBatchesBelowHighestTieredOffsetV1Test --no-daemon --no-parallelJAVA_HOME=$(/usr/libexec/java_home -v 17) ./gradlew :storage:checkstyleMain :storage:checkstyleTest :storage:inkless:checkstyleMain :storage:inkless:checkstyleTest --no-daemon --no-parallelbash tests/docker/ducker-ak test tests/kafkatest/tests/inkless/consolidation_cross_tier_earliest_test.py