Skip to content

[fix][test] Fix flaky shared read-ahead verification in ManagedLedgerTest - #27490

Open
void-ptr974 wants to merge 1 commit into
apache:masterfrom
void-ptr974:fix/shared-read-ahead-test
Open

void-ptr974 wants to merge 1 commit into
apache:masterfrom
void-ptr974:fix/shared-read-ahead-test

Conversation

@void-ptr974

@void-ptr974 void-ptr974 commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

This PR fixes a flaky assertion in cursorsAtTheSamePositionShareTheReadAhead and strengthens its behavioral checks. Both cursors must receive the correct entries and reuse the prefetched cache, with exactly one storage read per range.

The demand read is submitted first, but both mock reads reach the interceptor through thenComposeAsync, so their observation order is not guaranteed. CI observed [5, 0], which the test incorrectly rejected because it required [0, 5].

Modifications

Verify the shared read-ahead contract

  • Require starting entries 0 and 5 exactly once each, in either order. Missing, duplicate, or additional storage reads still fail.
  • Hold the first prefetch until the second cursor has read the initial range. Verify complete entry IDs and payloads, and cache-hit counts showing that both cursors reuse the prefetched entries.

Keep the fixture stable and clean up on failure

  • Use only two ranges to avoid unrelated background prefetches, and retain cached entries throughout the bounded waits to prevent time-based eviction from affecting the assertions.
  • Release the held response and all returned entries, and close the ledger even when an assertion fails.

Verifying this change

  • Make sure that the change passes the CI checks.

Validated on Linux x86_64 with JDK 21 and test retries disabled:

  • The changed method and four related read-ahead methods passed.
  • ./gradlew quickCheck passed.
  • Repeated validation of this test change passed 50 normal-scheduling invocations and 10 invocations forcing the legal [5, 0] order. The scheduling gates were temporary and are not included in this PR.
  • The original ordered assertion reproduced the CI failure under normal scheduling and failed all five controlled reverse-order invocations.

The changed test can be run with:

./gradlew :managed-ledger:test \
  --tests org.apache.bookkeeper.mledger.impl.ManagedLedgerTest.cursorsAtTheSamePositionShareTheReadAhead \
  -PtestRetryCount=0 -PtestFailFast=false

Does this pull request potentially affect one of the following parts:

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

…Test

Verify shared prefetch results and cache reuse without ordering asynchronous mock reads.
Release test resources on assertion failures.

Assisted-by: Codex
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.

2 participants