[GitHub] [pulsar] sandrzejczak opened a new pull request #10305: [Issue 8214][pulsar-client] Added pausing of new topic consumers to MultiTopicsConsumerImpl.

[GitBox <gi...@apache.org>]

sandrzejczak opened a new pull request #10305:
URL: https://github.com/apache/pulsar/pull/10305


   Fixes https://github.com/apache/pulsar/issues/8214
   
   ### Motivation
   
   A new consumer can be added to the `MultiTopicsConsumerImpl` in one of two ways:
   - By invoking the `subscribeAsync(String topicName, boolean createTopicIfDoesNotExist)` method manually.
   - On topic partition number change (relevant method: `subscribeIncreasedTopicPartitions(String topicName)`).
   
   Currently, only the second way ensures that the newly added consumers are paused if the parent consumer is paused. The first way will never pause the newly added consumers.
   
   Moreover, the unit test for the second way is not strict enough. If we modify the `subscribeIncreasedTopicPartitions(String topicName)` method not to pause the newly created consumers, the unit test still passes. Unit test method: `SimpleProducerConsumerTest#testMultiTopicsConsumerImplPause()`.
   
   ### Modifications
   
   - Added pausing of the newly created consumers to the flow of the `subscribeAsync(String topicName, boolean createTopicIfDoesNotExist)` method. The logic is the same as the existing logic in the  `subscribeIncreasedTopicPartitions(String topicName)` method.
   - Fixed the `SimpleProducerConsumerTest#testMultiTopicsConsumerImplPause()` test method, as it was too lenient. It has been renamed to `SimpleProducerConsumerTest#testMultiTopicsConsumerImplPauseForPartitionNumberChange()`
   
   
   ### Verifying this change
   
   This change added tests and can be verified as follows:
   
     - Added the `SimpleProducerConsumerTest#testMultiTopicsConsumerImplPauseForManualSubscription()` unit test method.
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API: no
     - The schema: no
     - The default values of configurations: no
     - The wire protocol: no
     - The rest endpoints: no
     - The admin cli options: no
     - Anything that affects deployment: no
   
   ### Documentation
   
     - Does this pull request introduce a new feature? no
     ~~- If yes, how is the feature documented? not applicable~~
     ~~- If a feature is not applicable for documentation, explain why?~~
     ~~- If a feature is not documented yet in this PR, please create a followup issue for adding the documentation~~
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] merlimat merged pull request #10305: [Issue 8214][pulsar-client] Added pausing of new topic consumers to MultiTopicsConsumerImpl.

[GitBox <gi...@apache.org>]

merlimat merged pull request #10305:
URL: https://github.com/apache/pulsar/pull/10305


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] eolivelli commented on pull request #10305: [Issue 8214][pulsar-client] Added pausing of new topic consumers to MultiTopicsConsumerImpl.

[GitBox <gi...@apache.org>]

eolivelli commented on pull request #10305:
URL: https://github.com/apache/pulsar/pull/10305#issuecomment-829022689


   This patch does not apply to branch-2.7


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org