lhotari opened a new pull request, #26122:
URL: https://github.com/apache/pulsar/pull/26122
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 #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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
