Skip to content

Conversation

@3pacccccc
Copy link
Contributor

Fixes #24850

Motivation

The test testReceiveAsyncCompletedWhenClosing was experiencing concurrency race conditions that caused flaky test failures. The issue occurred due to unpredictable timing between two operations:

  • consumer.receiveAsync() or batchReceiveAsync()

  • consumer.close()

Expected behavior:

  • receiveAsync() should happen before close(), allowing the async operation to be properly interrupted by the close.

Flaky scenario:

  • When close() executes before receiveAsync(), the subsequent receiveAsync() call immediately returns a CompletableFuture that completes with a PulsarClientException containing the message "Consumer already closed". This differs from the expected interruption behavior and causes the test assertion to fail.

Modifications

ensure consumer.receiveAsync() happens before consumer.close()

Solution

Added proper synchronization using CountDownLatch to ensure:

  • The async operation (receiveAsync/batchReceiveAsync) starts first

  • The close() operation executes only after the async operation has begun

Verifying this change

  • Make sure that the change passes the CI checks.

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

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: 3pacccccc#27

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 16, 2025
@codecov-commenter
Copy link

codecov-commenter commented Oct 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.15%. Comparing base (acad78c) to head (c427394).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #24858       +/-   ##
=============================================
+ Coverage     38.49%   74.15%   +35.65%     
- Complexity    13160    33437    +20277     
=============================================
  Files          1856     1913       +57     
  Lines        145149   149915     +4766     
  Branches      16844    17521      +677     
=============================================
+ Hits          55880   111172    +55292     
+ Misses        81652    29873    -51779     
- Partials       7617     8870     +1253     
Flag Coverage Δ
inttests 26.63% <ø> (-0.09%) ⬇️
systests 22.87% <ø> (+0.02%) ⬆️
unittests 73.62% <ø> (+39.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1412 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Technoboy- Technoboy- merged commit 1ca1797 into apache:master Oct 28, 2025
174 of 186 checks passed
lhotari pushed a commit that referenced this pull request Oct 28, 2025
lhotari pushed a commit that referenced this pull request Oct 28, 2025
manas-ctds pushed a commit to datastax/pulsar that referenced this pull request Oct 29, 2025
…etedWhenClosing (apache#24858)

(cherry picked from commit 1ca1797)
(cherry picked from commit ec89455)
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.

Flaky-test: SimpleProducerConsumerTest.testReceiveAsyncCompletedWhenClosing

5 participants