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

[Issue 8214][pulsar-client] Added pausing of new topic consumers to MultiTopicsConsumerImpl. #10305

Conversation

sandrzejczak
Copy link
Contributor

@sandrzejczak sandrzejczak commented Apr 21, 2021

Fixes #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 result in pausing 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:

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

@merlimat merlimat added the type/bug The PR fixed a bug or issue reported a bug label Apr 21, 2021
@merlimat merlimat added this to the 2.8.0 milestone Apr 21, 2021
@merlimat merlimat merged commit ed3e97b into apache:master Apr 21, 2021
@eolivelli
Copy link
Contributor

This patch does not apply to branch-2.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consumer pause does not work properly in case of multi topics consumers.
3 participants