Skip to content

[fix][test] Relax BrokerRegistryIntegrationTest broker-close threshold - #25562

Merged
nodece merged 1 commit into
apache:masterfrom
merlimat:fix/broker-registry-close-threshold
Apr 22, 2026
Merged

[fix][test] Relax BrokerRegistryIntegrationTest broker-close threshold#25562
nodece merged 1 commit into
apache:masterfrom
merlimat:fix/broker-registry-close-threshold

Conversation

@merlimat

Copy link
Copy Markdown
Contributor

Motivation

BrokerRegistryIntegrationTest and its subclass BrokerRegistryMetadataStoreIntegrationTest fail their @AfterClass cleanup method if pulsar.close() takes more than 5 seconds. On slower CI agents (or for the metadata-store variant), the broker shutdown can legitimately exceed 5 seconds, producing spurious failures.

The assertion was originally added by PR #23371 as a regression detector for multi-minute hangs caused by blocked health checks / stuck registrations. A 15-second threshold still catches those hangs but tolerates normal CI variability.

Example failure

org.apache.pulsar.broker.loadbalance.extensions.BrokerRegistryMetadataStoreIntegrationTest.cleanup FAILED
    java.lang.RuntimeException: Broker took 10173ms to close
        at org.apache.pulsar.broker.loadbalance.extensions.BrokerRegistryIntegrationTest.cleanup(BrokerRegistryIntegrationTest.java:74)
        ...
969 tests completed, 1 failed, 5 skipped

Modifications

Raise the cleanup-threshold from 5000ms to 15000ms. Add a comment explaining the intent so future readers understand why the check exists and why it is intentionally loose.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework of an existing test-only assertion and is already covered by existing tests (BrokerRegistryIntegrationTest, BrokerRegistryMetadataStoreIntegrationTest).

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

The test (and its BrokerRegistryMetadataStoreIntegrationTest subclass) fail the
cleanup method if pulsar.close() takes more than 5 seconds. On slower CI agents
the broker shutdown legitimately crosses this boundary (e.g. 10173ms observed),
producing spurious failures.

The assertion was added as a regression detector for multi-minute hangs (see
PR apache#23371). Raising the threshold to 15s still catches those hangs while
tolerating normal CI variability.
@nodece
nodece merged commit f26e3ba into apache:master Apr 22, 2026
43 checks passed
lhotari pushed a commit that referenced this pull request Apr 22, 2026
lhotari pushed a commit that referenced this pull request Apr 22, 2026
lhotari pushed a commit that referenced this pull request Apr 22, 2026
@merlimat
merlimat deleted the fix/broker-registry-close-threshold branch April 22, 2026 12:51
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 23, 2026
poorbarcode pushed a commit to poorbarcode/pulsar that referenced this pull request May 6, 2026
nodece pushed a commit to ascentstream/pulsar that referenced this pull request May 27, 2026
@lhotari lhotari added this to the 5.0.0-M1 milestone Jun 12, 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.

3 participants