fix(inkless:consolidation): serve cross-tier earliest offset promptly after startup#688
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness gap where ListOffsets(EARLIEST) for born-consolidated (cross-tier) topics could return the local log_start_offset for up to log.initial.task.delay.ms after broker startup, causing consumers with auto.offset.reset=earliest to skip remotely-readable data. It also adjusts the related test to avoid racing the asynchronous publication and improves Inkless CI reporting so failures are surfaced deterministically.
Changes:
- Schedule
inkless-cross-tier-log-start-reporterwith its own report interval as the initial delay/period (instead oflogInitialTaskDelayMs) so cross-tier EARLIEST converges promptly after startup. - Update
InklessConsolidatedDisklessTopicsTestto wait for EARLIEST offsets to converge before asserting. - Improve PR CI behavior by disabling test retries, emitting JUnit XML to a clean path, and parsing JUnit results into the job summary with
junit.py.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| storage/inkless/src/main/java/io/aiven/inkless/config/InklessConfig.java | Normalizes punctuation in config docs (ASCII-friendly). |
| server-common/src/main/java/org/apache/kafka/server/config/ServerConfigs.java | Normalizes punctuation in config docs (ASCII-friendly). |
| Makefile | Aligns local make build/test targets with CI task selection and test filters. |
| docs/inkless/configs.rst | Keeps rendered config documentation punctuation consistent with source docs. |
| core/src/test/scala/io/aiven/inkless/consolidation/DisklessLeaderEndPointTest.scala | Normalizes punctuation in test comments. |
| core/src/test/java/kafka/server/InklessConsolidatedDisklessTopicsTest.java | Avoids racing async cross-tier earliest-offset publication by polling for convergence. |
| core/src/main/scala/kafka/server/ReplicaManager.scala | Fixes reporter scheduling to publish cross-tier earliest offsets promptly after startup. |
| core/src/main/scala/io/aiven/inkless/consolidation/DisklessLeaderEndPoint.scala | Normalizes punctuation in scaladoc. |
| .github/workflows/inkless.yml | Makes CI fail deterministically on first failure and adds junit.py-based failure reporting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+464
to
+474
| TestUtils.waitForCondition(() -> { | ||
| try { | ||
| return queryBrokerEarliestOffsets(commonConfigs).values().stream() | ||
| .allMatch(offset -> Long.valueOf(expectedEarliest).equals(offset)); | ||
| } catch (ExecutionException | TimeoutException e) { | ||
| return false; | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| throw new RuntimeException(e); | ||
| } | ||
| }, 60_000, () -> "ListOffsets earliest for all partitions should converge to " + expectedEarliest + " " + context); |
giuseppelillo
previously approved these changes
Jul 8, 2026
…ly after startup Born-consolidated ListOffsets(EARLIEST) returns COALESCE(remote_log_start_offset, log_start_offset). The leader publishes remote_log_start_offset via CrossTierLogStartReporter, but the reporter's first flush was gated by log.initial.task.delay.ms (default 30s). Until it fired, EARLIEST returned the pruned log_start_offset instead of the true cross-tier earliest, so a consumer with auto.offset.reset=earliest could skip records still readable from remote for up to ~30s after every broker startup. Schedule the reporter with its own report interval as the initial delay (like the sibling inkless-file-cleaner / log-pruner tasks) instead of logInitialTaskDelayMs. InklessConsolidatedDisklessTopicsTest now polls until EARLIEST converges before the hard assertion, matching the reporter's async publish contract (the check previously raced the first flush and failed on fast runners, e.g. main).
…flaky PR failures Inkless CI logged only a raw gradle exit code on failure. Parse the JUnit XML with junit.py (as ci.yml/build.yml do) so the job summary lists failed tests, and let its exit code drive the outcome. Set kafka.test.xml.output.dir for clean report paths. Set maxTestRetries=0 so a single failure fails a PR to main instead of being retried away. Full build.yml parity on PRs to main stays intentionally out of scope: PR CI runs focalized tests to stay fast.
giuseppelillo
approved these changes
Jul 8, 2026
jeqo
added a commit
that referenced
this pull request
Jul 8, 2026
… after startup (#688) * chore(inkless): align make targets with inkless ci * chore(inkless): normalize ascii punctuation * fix(inkless:consolidation): report cross-tier log start offset promptly after startup Born-consolidated ListOffsets(EARLIEST) returns COALESCE(remote_log_start_offset, log_start_offset). The leader publishes remote_log_start_offset via CrossTierLogStartReporter, but the reporter's first flush was gated by log.initial.task.delay.ms (default 30s). Until it fired, EARLIEST returned the pruned log_start_offset instead of the true cross-tier earliest, so a consumer with auto.offset.reset=earliest could skip records still readable from remote for up to ~30s after every broker startup. Schedule the reporter with its own report interval as the initial delay (like the sibling inkless-file-cleaner / log-pruner tasks) instead of logInitialTaskDelayMs. InklessConsolidatedDisklessTopicsTest now polls until EARLIEST converges before the hard assertion, matching the reporter's async publish contract (the check previously raced the first flush and failed on fast runners, e.g. main). * chore(inkless:ci): report failed tests via junit.py and stop masking flaky PR failures Inkless CI logged only a raw gradle exit code on failure. Parse the JUnit XML with junit.py (as ci.yml/build.yml do) so the job summary lists failed tests, and let its exit code drive the outcome. Set kafka.test.xml.output.dir for clean report paths. Set maxTestRetries=0 so a single failure fails a PR to main instead of being retried away. Full build.yml parity on PRs to main stays intentionally out of scope: PR CI runs focalized tests to stay fast.
jeqo
added a commit
that referenced
this pull request
Jul 8, 2026
… after startup (#688) * chore(inkless): align make targets with inkless ci * chore(inkless): normalize ascii punctuation * fix(inkless:consolidation): report cross-tier log start offset promptly after startup Born-consolidated ListOffsets(EARLIEST) returns COALESCE(remote_log_start_offset, log_start_offset). The leader publishes remote_log_start_offset via CrossTierLogStartReporter, but the reporter's first flush was gated by log.initial.task.delay.ms (default 30s). Until it fired, EARLIEST returned the pruned log_start_offset instead of the true cross-tier earliest, so a consumer with auto.offset.reset=earliest could skip records still readable from remote for up to ~30s after every broker startup. Schedule the reporter with its own report interval as the initial delay (like the sibling inkless-file-cleaner / log-pruner tasks) instead of logInitialTaskDelayMs. InklessConsolidatedDisklessTopicsTest now polls until EARLIEST converges before the hard assertion, matching the reporter's async publish contract (the check previously raced the first flush and failed on fast runners, e.g. main). * chore(inkless:ci): report failed tests via junit.py and stop masking flaky PR failures Inkless CI logged only a raw gradle exit code on failure. Parse the JUnit XML with junit.py (as ci.yml/build.yml do) so the job summary lists failed tests, and let its exit code drive the outcome. Set kafka.test.xml.output.dir for clean report paths. Set maxTestRetries=0 so a single failure fails a PR to main instead of being retried away. Full build.yml parity on PRs to main stays intentionally out of scope: PR CI runs focalized tests to stay fast.
jeqo
added a commit
that referenced
this pull request
Jul 9, 2026
PR #688 removed all test retries from inkless.yml, exposing the CI to known upstream flakiness (e.g. ReplicaManagerTest). Split the retry policy by test ownership: kafka-owned suites are retried (maxTestRetries=1, maxTestRetryFailures=3), while inkless-owned tests (:storage:inkless and *Inkless*/*Diskless*/io.aiven.inkless.*) run without retries so our own flakiness stays visible. The mixed :core:test invocation is split accordingly. Also add kafka.server.RequestQuotaTest to the kafka-owned :core:test group. It iterates over every broker ApiKey, so it guards that each inkless-added API (e.g. ALTER_DISKLESS_SWITCH) is wired into the request-building/quota paths, shifting that check left from nightly to per-PR CI.
tvainika
pushed a commit
that referenced
this pull request
Jul 9, 2026
…le (#692) * fix(inkless:test): handle ALTER_DISKLESS_SWITCH in RequestQuotaTest RequestQuotaTest iterates over all ApiKeys.brokerApis and builds a request for each, but requestBuilder had no case for the inkless-added ALTER_DISKLESS_SWITCH key, hitting the fallthrough and throwing IllegalArgumentException. This deterministically failed testUnauthorizedThrottle, testUnthrottledClient and testResponseThrottleTime. * fix(inkless:ci): correct nightly JUnit report path The nightly parse step pointed junit.py at build/junit-xml/<java>, but copyTestXml writes to build/junit-xml/<module>/<java>. The glob matched nothing, so real test failures were reported as '0 tests' and the job failed only on the gradle exit code. Point at build/junit-xml so the recursive **/*.xml glob finds all module reports, matching inkless.yml. * fix(inkless:ci): retry kafka-owned tests, keep inkless tests unretried PR #688 removed all test retries from inkless.yml, exposing the CI to known upstream flakiness (e.g. ReplicaManagerTest). Split the retry policy by test ownership: kafka-owned suites are retried (maxTestRetries=1, maxTestRetryFailures=3), while inkless-owned tests (:storage:inkless and *Inkless*/*Diskless*/io.aiven.inkless.*) run without retries so our own flakiness stays visible. The mixed :core:test invocation is split accordingly. Also add kafka.server.RequestQuotaTest to the kafka-owned :core:test group. It iterates over every broker ApiKey, so it guards that each inkless-added API (e.g. ALTER_DISKLESS_SWITCH) is wired into the request-building/quota paths, shifting that check left from nightly to per-PR CI. * fix(inkless:ci): enforce the test timeout across the whole suite chain The test step chained gradle invocations with && but only wrapped the first call in 'timeout ${TIMEOUT_MINUTES}m', so a hang in any later suite ran untimed and never produced exit 124 (which gates the thread-dump upload). Wrap the entire chain in a single 'timeout ... bash -c' so the budget covers all suites and a hang anywhere is killed and reported. Applied to inkless.yml and inkless-nightly.yml.
jeqo
added a commit
that referenced
this pull request
Jul 20, 2026
…le (#692) * fix(inkless:test): handle ALTER_DISKLESS_SWITCH in RequestQuotaTest RequestQuotaTest iterates over all ApiKeys.brokerApis and builds a request for each, but requestBuilder had no case for the inkless-added ALTER_DISKLESS_SWITCH key, hitting the fallthrough and throwing IllegalArgumentException. This deterministically failed testUnauthorizedThrottle, testUnthrottledClient and testResponseThrottleTime. * fix(inkless:ci): correct nightly JUnit report path The nightly parse step pointed junit.py at build/junit-xml/<java>, but copyTestXml writes to build/junit-xml/<module>/<java>. The glob matched nothing, so real test failures were reported as '0 tests' and the job failed only on the gradle exit code. Point at build/junit-xml so the recursive **/*.xml glob finds all module reports, matching inkless.yml. * fix(inkless:ci): retry kafka-owned tests, keep inkless tests unretried PR #688 removed all test retries from inkless.yml, exposing the CI to known upstream flakiness (e.g. ReplicaManagerTest). Split the retry policy by test ownership: kafka-owned suites are retried (maxTestRetries=1, maxTestRetryFailures=3), while inkless-owned tests (:storage:inkless and *Inkless*/*Diskless*/io.aiven.inkless.*) run without retries so our own flakiness stays visible. The mixed :core:test invocation is split accordingly. Also add kafka.server.RequestQuotaTest to the kafka-owned :core:test group. It iterates over every broker ApiKey, so it guards that each inkless-added API (e.g. ALTER_DISKLESS_SWITCH) is wired into the request-building/quota paths, shifting that check left from nightly to per-PR CI. * fix(inkless:ci): enforce the test timeout across the whole suite chain The test step chained gradle invocations with && but only wrapped the first call in 'timeout ${TIMEOUT_MINUTES}m', so a hang in any later suite ran untimed and never produced exit 124 (which gates the thread-dump upload). Wrap the entire chain in a single 'timeout ... bash -c' so the budget covers all suites and a hang anywhere is killed and reported. Applied to inkless.yml and inkless-nightly.yml.
jeqo
added a commit
that referenced
this pull request
Jul 20, 2026
…le (#692) * fix(inkless:test): handle ALTER_DISKLESS_SWITCH in RequestQuotaTest RequestQuotaTest iterates over all ApiKeys.brokerApis and builds a request for each, but requestBuilder had no case for the inkless-added ALTER_DISKLESS_SWITCH key, hitting the fallthrough and throwing IllegalArgumentException. This deterministically failed testUnauthorizedThrottle, testUnthrottledClient and testResponseThrottleTime. * fix(inkless:ci): correct nightly JUnit report path The nightly parse step pointed junit.py at build/junit-xml/<java>, but copyTestXml writes to build/junit-xml/<module>/<java>. The glob matched nothing, so real test failures were reported as '0 tests' and the job failed only on the gradle exit code. Point at build/junit-xml so the recursive **/*.xml glob finds all module reports, matching inkless.yml. * fix(inkless:ci): retry kafka-owned tests, keep inkless tests unretried PR #688 removed all test retries from inkless.yml, exposing the CI to known upstream flakiness (e.g. ReplicaManagerTest). Split the retry policy by test ownership: kafka-owned suites are retried (maxTestRetries=1, maxTestRetryFailures=3), while inkless-owned tests (:storage:inkless and *Inkless*/*Diskless*/io.aiven.inkless.*) run without retries so our own flakiness stays visible. The mixed :core:test invocation is split accordingly. Also add kafka.server.RequestQuotaTest to the kafka-owned :core:test group. It iterates over every broker ApiKey, so it guards that each inkless-added API (e.g. ALTER_DISKLESS_SWITCH) is wired into the request-building/quota paths, shifting that check left from nightly to per-PR CI. * fix(inkless:ci): enforce the test timeout across the whole suite chain The test step chained gradle invocations with && but only wrapped the first call in 'timeout ${TIMEOUT_MINUTES}m', so a hang in any later suite ran untimed and never produced exit 124 (which gates the thread-dump upload). Wrap the entire chain in a single 'timeout ... bash -c' so the budget covers all suites and a hang anywhere is killed and reported. Applied to inkless.yml and inkless-nightly.yml.
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.
Fixes a regression where
InklessConsolidatedDisklessTopicsTest.testNewConsolidatedDisklessTopicsfailed onmain(ListOffsets earliest ... expected: <0> but was: <9>) while it had passed on the PR that introduced it. The failure is a real, if transient, production correctness gap for born-consolidated topics — not just a flaky test.Root cause
Born-consolidated
ListOffsets(EARLIEST)returnsCOALESCE(remote_log_start_offset, log_start_offset). The leader publishesremote_log_start_offsetasynchronously throughCrossTierLogStartReporter, but the reporter's first flush was scheduled withlog.initial.task.delay.msas its initial delay (default 30s).Until that first flush:
remote_log_start_offsetis NULL in the control plane, and the diskless WAL prune frontier has already advancedlog_start_offset(to9in the test), soCOALESCE(NULL, 9) = 9. A consumer withauto.offset.reset=earlieststarting in that window skips records that are still readable from remote — for up to ~30s after every broker startup.Why it was green on the PR but red on
main: the assertion races the 30s first flush. The slower PR CI runner happened to cross 30s before asserting; faster runners (andmain) do not. KC-171 (#679, consolidation fetch path) shifted tiering/prune timing enough to tip the pre-existing race into a consistent failure.Fix
ReplicaManager: scheduleinkless-cross-tier-log-start-reporterwith its own report interval as the initial delay (matching the siblinginkless-file-cleaner/inkless-consolidated-diskless-log-prunertasks) instead oflogInitialTaskDelayMs. EARLIEST now becomes correct within one report interval (1s default) after startup.InklessConsolidatedDisklessTopicsTest: poll until EARLIEST converges before the hard assertion, which matches the reporter's async publish contract instead of racing the first flush.CI changes (chore)
Bundled
chore(inkless:ci)improvements motivated by "green on PR, red on main":junit.py(asci.yml/build.ymldo) so the job summary lists the failed tests and its exit code drives the outcome; setkafka.test.xml.output.dirfor clean paths.maxTestRetries=0so a single failure fails a PR tomaininstead of being retried away.Full
build.ymlparity on PRs tomainis intentionally out of scope: PR CI runs focalized tests to stay fast; the full matrix runs post-merge.Testing
main(expected: <0> but was: <9>)../gradlew :core:test --tests "kafka.server.InklessConsolidatedDisklessTopicsTest" --rerun-tasks.Commits
chore(inkless): align make targets with inkless cichore(inkless): normalize ascii punctuationfix(inkless:consolidation): report cross-tier log start offset promptly after startupchore(inkless:ci): report failed tests via junit.py and stop masking flaky PR failures