Skip to content

[fix][test] Avoid unsafe cleanup between shared-cluster tests - #26611

Merged
merlimat merged 3 commits into
apache:masterfrom
lhotari:lh-fix-test-cleanup-listeners
Sep 17, 2026
Merged

merlimat merged 3 commits into
apache:masterfrom
lhotari:lh-fix-test-cleanup-listeners

Conversation

@lhotari

@lhotari lhotari commented Sep 16, 2026

Copy link
Copy Markdown
Member

Motivation

SharedPulsarCluster keeps broker resources and worker threads alive across test classes. FastThreadLocalCleanupListener removes Netty fast-thread-local state between classes, while MockitoCleanupListener reflects into live workers' thread-local maps and performs global inline-mock cleanup. These operations can interfere with resources still owned by the shared cluster.

Global mock cleanup also does not solve ownership: Mockito invocations on a long-lived worker can retain ongoing-stubbing state and its mock graph on that worker. Clearing mocks from a different thread does not remove those references. Tests should avoid invoking mocks on shared workers and explicitly release fixtures they own.

Modifications

  • Remove FastThreadLocalCleanupListener and MockitoCleanupListener from the default TestNG listeners and delete the listener implementations. Keep the cleaner utilities for explicitly scoped use.
  • Replace shared-worker mocks in dispatcher, reconnection, I/O, TTL, entry-filter, and entry-release tests with real fixtures or small subclasses.
  • Add package-private testing helpers for metadata parsing/release and the TTL fallback path, preserving production behavior. Exercise metadata success and failure paths with real entries and buffers.
  • Add explicit cleanup for owned mocks and WebSocket annotation fixtures, restore the shared broker interceptor, and close real Netty channels and their event loop.
  • Update the multi-topic close test to monitor the scheduled executor provider actually used for retries.

Verifying this change

  • Local ./gradlew spotlessCheck checkstyleMain checkstyleTest and ./gradlew quickCheck passed.
  • 82 scoped cases across the 13 affected test classes passed with retries disabled: dispatcher permit accounting and read conflation, single-active and classic dispatchers, metadata entry release, reader TTL, subscription and broker stats, multi-topic close, producer reconnection, I/O failures, Netty futures, and WebSocket authorization.
  • 12 scoped buildtools cases passed for the listener-removal change.
  • Make sure that the change passes the CI checks.

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

Shared Pulsar clusters keep their worker threads alive between test classes. FastThreadLocal cleanup modifies those live threads without synchronization, and Mockito cleanup similarly mutates their internal ThreadLocal maps. Remove both listeners and their default registrations to avoid racing shared infrastructure.

Keep explicit cleaner utilities available. Other registered listeners do not perform equivalent cross-thread state cleanup; shared clusters already reset the thread leak detector baseline.

Validation: ./gradlew quickCheck and 12 tests from BetweenTestClassesListenerAdapterTest and FastThreadLocalStateCleanerTest with testRetryCount=0.

Assisted-by: Codex
Replace shared-worker mocks with real cursors, channels, entry filters, and narrow subclasses. Test metadata entry release and TTL fallback through package-private helpers instead of modifying shared broker internals.

Explicitly release test-owned mocks and annotation fixtures, restore the broker interceptor, and monitor the executor actually used for multi-topic retries.

Validation: quickCheck and 82 scoped test cases passed with retries disabled.

Assisted-by: Codex
@lhotari lhotari added this to the 5.0.0 milestone Sep 16, 2026
The removed listener was its only caller. Delete the cleaner and its tests, and update the Netty dependency comment.

Assisted-by: Codex
@merlimat
merlimat merged commit 136cdf6 into apache:master Sep 17, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants