Skip to content

[fix][test] Fix flaky OffloadPrefixTest.testPositionOnEdgeOfLedger race with ledger rollover#25561

Merged
lhotari merged 1 commit intoapache:masterfrom
merlimat:fix/flaky-offload-prefix-test
Apr 22, 2026
Merged

[fix][test] Fix flaky OffloadPrefixTest.testPositionOnEdgeOfLedger race with ledger rollover#25561
lhotari merged 1 commit intoapache:masterfrom
merlimat:fix/flaky-offload-prefix-test

Conversation

@merlimat
Copy link
Copy Markdown
Contributor

Motivation

OffloadPrefixTest.testPositionOnEdgeOfLedger is flaky. The test writes exactly 20 entries with maxEntriesPerLedger=10, which fills two ledgers. Once the second ledger is full, it is closed and a third (empty) ledger starts being created asynchronously on the managed-ledger executor. The test's next line asserts that only 2 ledgers exist — this races with the async rollover and the test intermittently fails with a third ledger already present.

Example failure

Gradle suite > Gradle test > org.apache.bookkeeper.mledger.impl.OffloadPrefixTest > testPositionOnEdgeOfLedger FAILED
    java.lang.AssertionError: expected [2] but found [3]
        at org.testng.Assert.fail(Assert.java:110)
        at org.testng.Assert.failNotEquals(Assert.java:1577)
        at org.testng.Assert.assertEqualsImpl(Assert.java:149)
        at org.testng.Assert.assertEquals(Assert.java:131)
        at org.testng.Assert.assertEquals(Assert.java:1418)
        at org.testng.Assert.assertEquals(Assert.java:1382)
        at org.testng.Assert.assertEquals(Assert.java:1428)
        at org.apache.bookkeeper.mledger.impl.OffloadPrefixTest.testPositionOnEdgeOfLedger(OffloadPrefixTest.java:237)

Modifications

Replace the hard-coded assertEquals(size, 2) with an Awaitility.await() that waits for the 3-ledger steady state (l1 full, l2 full, l3 empty). The rest of the test (getLastConfirmedEntry returning the last entry of l2, then addEntry("entry-blah") being stored in l3, then the two offloadPrefix calls) already works correctly against this stable state, so no other changes are required.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as OffloadPrefixTest.testPositionOnEdgeOfLedger.

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

…ce with ledger rollover

After writing exactly 20 entries with maxEntriesPerLedger=10, the second ledger
is closed and a third empty ledger is created asynchronously on the managed
ledger executor. The test assertion that the ledger list size is 2 races with
that async creation, so the test intermittently sees 3 ledgers and fails.

Wait for the rollover to finish (3 ledgers: l1 full, l2 full, l3 empty) before
proceeding; all other assertions in the test already work correctly with this
stable state.
@lhotari lhotari merged commit 77999fc into apache:master Apr 22, 2026
43 checks passed
lhotari pushed a commit that referenced this pull request Apr 22, 2026
…ce with ledger rollover (#25561)

(cherry picked from commit 77999fc)
lhotari pushed a commit that referenced this pull request Apr 22, 2026
…ce with ledger rollover (#25561)

(cherry picked from commit 77999fc)
lhotari pushed a commit that referenced this pull request Apr 22, 2026
…ce with ledger rollover (#25561)

(cherry picked from commit 77999fc)
@merlimat merlimat deleted the fix/flaky-offload-prefix-test branch April 22, 2026 12:50
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 23, 2026
…ce with ledger rollover (apache#25561)

(cherry picked from commit 77999fc)
(cherry picked from commit 6f57b19)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants