You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@pulsar.apache.org by "lhotari (via GitHub)" <gi...@apache.org> on 2026/07/27 07:20:59 UTC

[PR] [fix][test] Fix flaky AdminApiTest caused by leaked brokerShutdownTimeoutMs [pulsar]

lhotari opened a new pull request, #26249:
URL: https://github.com/apache/pulsar/pull/26249

### Motivation

`AdminApiTest` can fail with the whole class collapsing after a single test method: the
`@AfterMethod` (`resetClusters`) fails with `Connection refused` and TestNG then skips every
remaining test method of the class.

The same failure was observed on `branch-4.0` in
[`V1AdminApiTest`](https://github.com/apache/pulsar/actions/runs/30219147986/job/89839199357),
which is the copy of this class that PIP-457 (#25304) removed from master. The bug itself is still
present in `AdminApiTest` on master.

```
[ERROR] Tests run: 47, Failures: 1, Errors: 0, Skipped: 20 <<< FAILURE! - in ...V1AdminApiTest
[ERROR] V1AdminApiTest.reset:193 » PulsarAdmin ...RetryException: Could not complete the
operation. Number of retries has been exhausted.
Failed reason: Connection refused: localhost/[0:0:0:0:0:0:0:1]:36193
```

The causal chain:

1. `testGetDynamicLocalConfiguration` lowers the `brokerShutdownTimeoutMs` **dynamic**
configuration to `10` ms and never restores it. It only asserts the stored value; the broker
applies dynamic configuration changes asynchronously, so whether the value has reached
`pulsar.getConfiguration()` by the time the next test method runs is a race — that is where the
flakiness comes from.
2. The next test method in alphabetical order, `testInvalidDynamicConfigContentInMetadata`, starts
with `stopBroker()`. With a 10 ms budget `PulsarService#close` fails:

```
PulsarService - Shutdown timed out after 10 ms
org.apache.pulsar.broker.PulsarServerException:
org.apache.pulsar.common.util.FutureUtil$LowOverheadTimeoutException: Timeout in close
at org.apache.pulsar.broker.PulsarService.close(PulsarService.java:478)
at ...MockedPulsarServiceBaseTest.stopBroker(MockedPulsarServiceBaseTest.java:391)
```

3. `MockedPulsarServiceBaseTest.stopBroker()` runs `pulsar.close(); pulsar = null;`, so the thrown
exception skips the assignment and the test is left holding a broker that is no longer
listening, together with the `admin` client pointing at its web service port.
4. `resetClusters()` then calls `admin.tenants().getTenants()` and fails with `Connection refused`.
Because it is a configuration method, TestNG skips all remaining test methods of the class
(19 in the CI run above) and the build fails. `TestRetrySupport` does recreate the test context
for the retry, and the retried test passes, but the configuration failure has already been
recorded.

### Modifications

In `AdminApiTest`:

- Add `restoreBrokerShutdownTimeout()` and call it at the start of `resetClusters()`. It removes
the `brokerShutdownTimeoutMs` dynamic configuration (which makes the broker restore the value it
was started with) and waits until the value the broker uses is no longer the override, so no
test method can leave a ~10 ms shutdown budget behind for a later `stopBroker()` /
`restartBroker()`.
- Return early from `resetClusters()` when the broker isn't running, with a warning. Throwing from
a configuration method skips every remaining test method of the class; `TestRetrySupport`
already recreates the shared test context before the next test method runs.
- Make `testGetDynamicLocalConfiguration` wait until the broker has applied the value it sets.
This is a small strengthening of the test itself, and it turns the race into a deterministic
case that the `@AfterMethod` has to handle, so a regression fails consistently instead of
flaking.

No production code is changed.

### Verifying this change

- [x] Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as *`AdminApiTest`*.

The failure was reproduced deterministically before fixing it, by making the asynchronous
propagation in `testGetDynamicLocalConfiguration` explicit and running
`testGetDynamicLocalConfiguration`, `testInvalidDynamicConfigContentInMetadata` and
`testJacksonWithTypeDifferencies` together. On master that run fails with exactly the CI
signature — `FutureUtil$LowOverheadTimeoutException: Timeout in close` on the test method and
`PulsarAdminException ... Connection refused` on `resetClusters`, with the following test method
skipped — and it passes with this change.

The full `AdminApiTest` class was run before and after the change: 100 tests, 0 failures in both
runs. The single retried `testUpdateDynamicLoadBalancerSheddingIntervalMinutes` behaves identically
on unmodified master (it asserts on the `conf` field that `resetConfig()` replaces in every
`@AfterMethod`, so it only passes once `TestRetrySupport` has re-run the setup); it is unrelated to
this change.

`branch-4.0` still carries `V1AdminApiTest` with the same problem and will need a separate fix; it
cannot inherit this one.

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

*If the box was checked, please highlight the changes*

- [ ] 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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][test] Fix flaky AdminApiTest caused by leaked brokerShutdownTimeoutMs [pulsar]

Posted by "nodece (via GitHub)" <gi...@apache.org>.

nodece merged PR #26249:
URL: https://github.com/apache/pulsar/pull/26249


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org