Skip to content

[fix][test] Recreate EventLoop in PublishRateLimiterTest setup - #25560

Merged
merlimat merged 1 commit into
apache:masterfrom
merlimat:fix-publish-rate-limiter-test
Apr 21, 2026
Merged

[fix][test] Recreate EventLoop in PublishRateLimiterTest setup#25560
merlimat merged 1 commit into
apache:masterfrom
merlimat:fix-publish-rate-limiter-test

Conversation

@merlimat

Copy link
Copy Markdown
Contributor

Summary

PublishRateLimiterTest has a latent bug: the class-level eventLoop
field is initialized once (at instance construction), but
@AfterMethod tearDown() calls eventLoop.shutdownGracefully() after
every test. TestNG reuses a single instance per class, so the second
test method to run in the class hits
RejectedExecutionException: event executor terminated when it calls
eventLoop.execute(...).

Two recent additions to the class
(shouldUnthrottleImmediatelyAfterDisablingLimitsDespiteLongPendingDelay
and shouldUnthrottleImmediatelyAfterRaisingByteLimitDespiteLongPendingDelay,
added in #25502) exposed the latent bug — in TestNG's alphabetical
default order, they run before testPublishRateLimiterImplExceed, and
their tearDown terminates the shared event loop.

The fix: move eventLoop creation into @BeforeMethod so each test
gets a fresh instance. @AfterMethod tearDown() continues to shut
it down at the end of each test.

Test plan

  • ./gradlew :pulsar-broker:test --tests "org.apache.pulsar.broker.service.PublishRateLimiterTest"
    all 4 tests pass.

The class-level `eventLoop` field was created once at instance construction
and shut down by `@AfterMethod tearDown()`, but never re-created. TestNG
reuses a single class instance across methods, so the second test to run
in the class hit `RejectedExecutionException: event executor terminated`
when it called `eventLoop.execute(...)`.

Two recent additions to this class (`shouldUnthrottleImmediately…`)
exposed the latent bug — in alphabetical TestNG order, they run before
`testPublishRateLimiterImplExceed` and their `tearDown` terminates the
shared event loop.

Move `eventLoop` creation into `@BeforeMethod` so each test gets a fresh
one.
@merlimat
merlimat merged commit 293eafc into apache:master Apr 21, 2026
43 checks passed
lhotari pushed a commit that referenced this pull request Apr 21, 2026
lhotari pushed a commit that referenced this pull request Apr 21, 2026
nodece pushed a commit to ascentstream/pulsar that referenced this pull request May 27, 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.

2 participants