Fixed reader reading from a partition #3960
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sijie
approved these changes
Apr 1, 2019
|
run java8 tests |
1 similar comment
|
run java8 tests |
merlimat
added a commit
that referenced
this pull request
Apr 2, 2019
1 task
BewareMyPower
added a commit
that referenced
this pull request
Sep 2, 2021
…11802) ### Motivation #3960 fixed the bug that reader will get stuck if it's reading a partition of a partitioned topic. The fix is using `isDurable` to check whether the consumer is a reader's internal consumer because it used `partitionIndex` to check whether the target topic is a partition while reader's `partitionIndex` is already set. However, for a non-durable multi-topics consumer, `isDurable` is false and each internal consumer will send FLOW request once the connection is established, which is unnecessary because `MultiTopicsConsumerImpl#startReceivingMessages` will send FLOW requests for each internal consumer after all internal consumers are connected. After #4591 introduced `hasParentConsumer` field, the check works for even a reader without the `isDurable` check. ### Modifications - Remove the check for `isDurable` before sending FLOW request and update the related comment. - Add a test for non-durable multi-topics consumer to verify the number of FLOW requests is the topics number, not the twice the topics number. ### Verifying this change - [x] Make sure that the change passes the CI checks. This change added `NonDurableSubscriptionTest#testFlowCountForMultiTopics` and the existing test `ReaderTest#testReadFromPartition` added in #3960 can also pass after this change. This change added tests and can be verified as follows:
codelipenghui
pushed a commit
that referenced
this pull request
Sep 9, 2021
…11802) ### Motivation #3960 fixed the bug that reader will get stuck if it's reading a partition of a partitioned topic. The fix is using `isDurable` to check whether the consumer is a reader's internal consumer because it used `partitionIndex` to check whether the target topic is a partition while reader's `partitionIndex` is already set. However, for a non-durable multi-topics consumer, `isDurable` is false and each internal consumer will send FLOW request once the connection is established, which is unnecessary because `MultiTopicsConsumerImpl#startReceivingMessages` will send FLOW requests for each internal consumer after all internal consumers are connected. After #4591 introduced `hasParentConsumer` field, the check works for even a reader without the `isDurable` check. ### Modifications - Remove the check for `isDurable` before sending FLOW request and update the related comment. - Add a test for non-durable multi-topics consumer to verify the number of FLOW requests is the topics number, not the twice the topics number. ### Verifying this change - [x] Make sure that the change passes the CI checks. This change added `NonDurableSubscriptionTest#testFlowCountForMultiTopics` and the existing test `ReaderTest#testReadFromPartition` added in #3960 can also pass after this change. This change added tests and can be verified as follows: (cherry picked from commit 1303e7d)
bharanic-dev
pushed a commit
to bharanic-dev/pulsar
that referenced
this pull request
Mar 18, 2022
…pache#11802) ### Motivation apache#3960 fixed the bug that reader will get stuck if it's reading a partition of a partitioned topic. The fix is using `isDurable` to check whether the consumer is a reader's internal consumer because it used `partitionIndex` to check whether the target topic is a partition while reader's `partitionIndex` is already set. However, for a non-durable multi-topics consumer, `isDurable` is false and each internal consumer will send FLOW request once the connection is established, which is unnecessary because `MultiTopicsConsumerImpl#startReceivingMessages` will send FLOW requests for each internal consumer after all internal consumers are connected. After apache#4591 introduced `hasParentConsumer` field, the check works for even a reader without the `isDurable` check. ### Modifications - Remove the check for `isDurable` before sending FLOW request and update the related comment. - Add a test for non-durable multi-topics consumer to verify the number of FLOW requests is the topics number, not the twice the topics number. ### Verifying this change - [x] Make sure that the change passes the CI checks. This change added `NonDurableSubscriptionTest#testFlowCountForMultiTopics` and the existing test `ReaderTest#testReadFromPartition` added in apache#3960 can also pass after this change. This change added tests and can be verified as follows:
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
When reader is reading from a partition in a partitioned topic, is currently getting stuck. The reason is that the initial batch of permits is never sent to broker.