Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][test] Fix SimpleProducerConsumerTest.testMultiTopicsConsumerImplPauseForManualSubscription #23546

Merged

Conversation

ZhaoGuorui666
Copy link
Contributor

Fixes #23485

Motivation

After calling pause(), there are still messages retained in incomingMessages because the internal consumer has already pre fetched a message and placed it in incomingMessages.

When calling pause(), these pre fetched messages are not immediately cleared, causing instability as messages can still be received during testing.

Modifications

A larger receiverQueueSize allows more messages to be prefetched and cached, ensuring that messages in the buffer are effectively managed after a pause operation, without causing message interference during testing.

In a multi topic consumer environment, the receiverQueueSize should be set reasonably. For multi topic consumers, the receiver Queue Size should be large enough to ensure that each internal ConsumerImpl instance can independently perform message prefetching without interfering with each other.

receiverQueueSize 1 -> 6

Verifying this change

image

Documentation

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

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Nov 3, 2024
@nodece nodece self-requested a review November 5, 2024 10:18
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.32%. Comparing base (bbc6224) to head (2cd1dfb).
Report is 713 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23546      +/-   ##
============================================
+ Coverage     73.57%   74.32%   +0.75%     
- Complexity    32624    34331    +1707     
============================================
  Files          1877     1943      +66     
  Lines        139502   147050    +7548     
  Branches      15299    16209     +910     
============================================
+ Hits         102638   109299    +6661     
- Misses        28908    29313     +405     
- Partials       7956     8438     +482     
Flag Coverage Δ
inttests 27.37% <ø> (+2.78%) ⬆️
systests 24.32% <ø> (-0.01%) ⬇️
unittests 73.72% <ø> (+0.87%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 654 files with indirect coverage changes

@Technoboy- Technoboy- added this to the 4.1.0 milestone Nov 6, 2024
@Technoboy- Technoboy- merged commit c4ee362 into apache:master Nov 6, 2024
54 checks passed
visxu pushed a commit to vissxu/pulsar that referenced this pull request Nov 6, 2024
lhotari pushed a commit that referenced this pull request Nov 13, 2024
…lPauseForManualSubscription (#23546)

(cherry picked from commit c4ee362)
lhotari pushed a commit that referenced this pull request Nov 13, 2024
…lPauseForManualSubscription (#23546)

(cherry picked from commit c4ee362)
lhotari pushed a commit that referenced this pull request Nov 13, 2024
…lPauseForManualSubscription (#23546)

(cherry picked from commit c4ee362)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 20, 2024
…lPauseForManualSubscription (apache#23546)

(cherry picked from commit c4ee362)
(cherry picked from commit b5c1c1c)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 21, 2024
…lPauseForManualSubscription (apache#23546)

(cherry picked from commit c4ee362)
(cherry picked from commit b5c1c1c)
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.testMultiTopicsConsumerImplPauseForManualSubscription
4 participants