Skip to content

[fix][test] Fix flaky AuditorBookieTest.testBookieClusterRestart - #26122

Merged
merlimat merged 2 commits into
apache:masterfrom
lhotari:lh-fix-flaky-auditorbookietest
Jul 1, 2026
Merged

[fix][test] Fix flaky AuditorBookieTest.testBookieClusterRestart#26122
merlimat merged 2 commits into
apache:masterfrom
lhotari:lh-fix-flaky-auditorbookietest

Conversation

@lhotari

@lhotari lhotari commented Jun 30, 2026

Copy link
Copy Markdown
Member

Fixes: #21538

Motivation

AuditorBookieTest.testBookieClusterRestart is flaky (issue #21538). Investigating a recent failure showed two independent problems that combine to make the test fail intermittently:

  1. Bookie cookie mismatch on restart. The test "restarts" the cluster with stopBKCluster() followed by startBKCluster(). startBKCluster() recreates the bookies with fresh data directories and pulls new ports from PortManager. PortManager can hand back a port a just-stopped bookie used, so a new bookie may come up on a recycled host:port with a different data directory. That fails bookie cookie validation against the cookie still registered in the metadata store, throwing BookieException$InvalidCookieException from startBKCluster():

    org.apache.bookkeeper.bookie.BookieException$InvalidCookieException: Cookie [...
    bookieHost: "127.0.0.1:42989"
    journalDir: "/tmp/bookie14854439573575621802test" ...]
    is not matching with [...
    bookieHost: "127.0.0.1:42989"
    journalDir: "/tmp/bookie10380846176335766468test" ...]
    
  2. Stale auditor electors leaking across runs. stopAuditorElectors() shuts the electors down but never clears the auditorElectors map. The same test instance is reused across test methods (and across TestNG retries), so a later run that iterates over auditorElectors observes stale, already-stopped electors whose isRunning() returns false, tripping the assertion:

    java.lang.AssertionError: Auditor elector is not running\!
        at org.apache.bookkeeper.replication.AuditorBookieTest.testBookieClusterRestart(AuditorBookieTest.java:133)
    

    This is the assertion reported in Flaky-test: AuditorBookieTest.testBookieClusterRestart #21538. It surfaced on a retry: the first attempt failed with the cookie mismatch (1), and the retry — running on the same instance with the previous run's shut-down electors still in the map — failed on this assertion in a few milliseconds.

Modifications

  • Restart the cluster with the existing restartBookies() helper, which preserves each bookie's identity (host:port and data directories) so cookie validation passes. This also drops the no-longer-needed metadataServiceUri juggling that startBKCluster() required.
  • Clear the auditorElectors map in stopAuditorElectors() so stale, shut-down electors are not re-checked by a subsequent test method or retry.

Verifying this change

This change is already covered by the existing AuditorBookieTest.

It was verified by running the class with @Test(invocationCount = 10) and -PtestRetryCount=0, which reproduces the same-instance retry scenario from CI: all 10 invocations of testBookieClusterRestart pass (0 failures, 0 skipped). spotlessCheck, checkstyleMain, and checkstyleTest pass.

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

### Motivation

`AuditorBookieTest.testBookieClusterRestart` is flaky. Two independent
problems combine to make it fail intermittently:

1. The "cluster restart" tears the cluster down with `stopBKCluster()` and
   recreates the bookies via `startBKCluster()`, which allocates fresh data
   directories and pulls ports from `PortManager`. `PortManager` can hand
   back a port a just-stopped bookie used, so a new bookie may land on a
   recycled host:port with a different data directory. That fails bookie
   cookie validation against the cookie still registered in the metadata
   store: `BookieException$InvalidCookieException`.

2. `stopAuditorElectors()` shuts the electors down but never clears the
   `auditorElectors` map. The same test instance is reused across test
   methods (and across TestNG retries), so a later run that iterates over
   `auditorElectors` observes stale, already-stopped electors whose
   `isRunning()` returns false, tripping the
   "Auditor elector is not running\!" assertion.

### Modifications

- Restart the cluster with `restartBookies()`, which preserves each bookie's
  identity (host:port and data dirs) so cookie validation passes. This also
  drops the no-longer-needed `metadataServiceUri` juggling.
- Clear the `auditorElectors` map in `stopAuditorElectors()` so stale
  electors are not re-checked by a subsequent test method or retry.

### Verification

Ran the class with `@Test(invocationCount = 10)` and `-PtestRetryCount=0`
(which reproduces the same-instance retry scenario from CI): all 10
invocations pass.

Assisted-by: Claude Code (Opus 4.8)
@merlimat
merlimat merged commit 116212f into apache:master Jul 1, 2026
42 checks passed
RobertIndie pushed a commit that referenced this pull request Jul 1, 2026
lhotari added a commit that referenced this pull request Jul 2, 2026
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.

Flaky-test: AuditorBookieTest.testBookieClusterRestart

3 participants