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
[pulsar-client] Fix the wrong multi-topic has message available behavior #13634
[pulsar-client] Fix the wrong multi-topic has message available behavior #13634
Conversation
/pulsarbot rerun-failure-checks |
4d42722
to
df2ec0e
Compare
/pulsarbot rerun-failure-checks |
2 similar comments
/pulsarbot rerun-failure-checks |
/pulsarbot rerun-failure-checks |
Look like the flaky test is not relevant to this PR. If I understand correctly. |
/pulsarbot rerun-failure-checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #13605 ### Motivation Currently, the multiTopicReader `hasMessageAvailable` might get the wrong result, we must check `numMessagesInQueue() > 0` again after finish all consumer `hasMessageAvaliableAsync` future, bacause some message might already in `MultiTopicsConsumerImpl#incomingMessages`. ### Modifications * Fix the wrong multi-topic has message available behavior. * Use `reader.readNextAsync()` instead of block method `reader.readNext()`. * Reduce the units test running time by changing `MultiTopicsReaderTest` to use `@BeforeClass`, `@AfterClass`. (cherry picked from commit e57dc8f)
Fixes #13605 ### Motivation Currently, the multiTopicReader `hasMessageAvailable` might get the wrong result, we must check `numMessagesInQueue() > 0` again after finish all consumer `hasMessageAvaliableAsync` future, bacause some message might already in `MultiTopicsConsumerImpl#incomingMessages`. ### Modifications * Fix the wrong multi-topic has message available behavior. * Use `reader.readNextAsync()` instead of block method `reader.readNext()`. * Reduce the units test running time by changing `MultiTopicsReaderTest` to use `@BeforeClass`, `@AfterClass`. (cherry picked from commit e57dc8f)
Fixes #13605
Motivation
Currently, the multiTopicReader
hasMessageAvailable
might get the wrong result, we must checknumMessagesInQueue() > 0
again after finish all consumerhasMessageAvaliableAsync
future, bacause some message might already inMultiTopicsConsumerImpl#incomingMessages
.Modifications
reader.readNextAsync()
instead of block methodreader.readNext()
.MultiTopicsReaderTest
to use@BeforeClass
,@AfterClass
.Documentation
Need to update docs?
no-need-doc
This is a bug fix, no need doc.