Skip to content

fix(inkless:consolidation): serve cross-tier earliest offset promptly after startup#688

Merged
giuseppelillo merged 4 commits into
mainfrom
jeqo/fix-regression
Jul 8, 2026
Merged

fix(inkless:consolidation): serve cross-tier earliest offset promptly after startup#688
giuseppelillo merged 4 commits into
mainfrom
jeqo/fix-regression

Conversation

@jeqo

@jeqo jeqo commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Fixes a regression where InklessConsolidatedDisklessTopicsTest.testNewConsolidatedDisklessTopics failed on main (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) returns COALESCE(remote_log_start_offset, log_start_offset). The leader publishes remote_log_start_offset asynchronously through CrossTierLogStartReporter, but the reporter's first flush was scheduled with log.initial.task.delay.ms as its initial delay (default 30s).

Until that first flush: remote_log_start_offset is NULL in the control plane, and the diskless WAL prune frontier has already advanced log_start_offset (to 9 in the test), so COALESCE(NULL, 9) = 9. A consumer with auto.offset.reset=earliest starting 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 (and main) 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: schedule inkless-cross-tier-log-start-reporter with its own report interval as the initial delay (matching the sibling inkless-file-cleaner / inkless-consolidated-diskless-log-pruner tasks) instead of logInitialTaskDelayMs. 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":

  • Report failed tests via junit.py (as ci.yml/build.yml do) so the job summary lists the failed tests and its exit code drives the outcome; set kafka.test.xml.output.dir for clean 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 is intentionally out of scope: PR CI runs focalized tests to stay fast; the full matrix runs post-merge.

Testing

  • Reproduced the failure deterministically on main (expected: <0> but was: <9>).
  • Confirmed the feature works with a prompt reporter, then verified the full class passes with the final fix: ./gradlew :core:test --tests "kafka.server.InklessConsolidatedDisklessTopicsTest" --rerun-tasks.

Commits

  • 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
  • chore(inkless:ci): report failed tests via junit.py and stop masking flaky PR failures

@jeqo
jeqo marked this pull request as ready for review July 8, 2026 09:45
@jeqo
jeqo requested a review from giuseppelillo July 8, 2026 10:27
@giuseppelillo
giuseppelillo requested a review from Copilot July 8, 2026 12:15

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 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-reporter with its own report interval as the initial delay/period (instead of logInitialTaskDelayMs) so cross-tier EARLIEST converges promptly after startup.
  • Update InklessConsolidatedDisklessTopicsTest to 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
giuseppelillo previously approved these changes Jul 8, 2026
jeqo added 2 commits July 8, 2026 15:49
…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
giuseppelillo merged commit 878ac51 into main Jul 8, 2026
5 checks passed
@giuseppelillo
giuseppelillo deleted the jeqo/fix-regression branch July 8, 2026 13:30
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.
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