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

support max consumers per subscription on topic level #8003

Conversation

hangc0276
Copy link
Contributor

Modifications

Support set max consumers per subscription on topic level.
Support get max consumers per subscription on topic level.
Support remove max consumers per subscription on topic level.

Verifying this change

This change added tests and can be verified as follows:

  • test set topic max consumers per subscription
  • test remove topic max consumers per subscription
  • test get topic topic max consumers per subscription
  • test disabled topic max consumers per subscription

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: (don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes)
  • Anything that affects deployment: (no)

Documentation

Does this pull request introduce a new feature? (yes)
If yes, how is the feature documented? (docs / JavaDocs)

@hangc0276 hangc0276 force-pushed the add_max_consumers_per_subscription_on_topic branch from a729baa to 11b3f9e Compare September 8, 2020 04:24
@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

2 similar comments
@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just left a minor comment

* @param topicName
* @return TopicPolicies is exist else return null.
*/
private TopicPolicies getTopicPolicies(TopicName topicName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks dup withNonPersistentDispatcherSingleActiveConsumer.java.

* @param topicName
* @return TopicPolicies is exist else return null.
*/
private TopicPolicies getTopicPolicies(TopicName topicName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the above comment, I think we can move this method to the BrokerService?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your reply. I have move the getTopicPolicies method to BrokerService, Please take a look again.

@codelipenghui codelipenghui added this to the 2.7.0 milestone Sep 9, 2020
@codelipenghui codelipenghui added component/topic-policy doc-required Your PR changes impact docs and you will update later. labels Sep 9, 2020
@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

3 similar comments
@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit e4b12df into apache:master Sep 11, 2020
lbenc135 pushed a commit to lbenc135/pulsar that referenced this pull request Oct 3, 2020
### Modifications
Support set max consumers per subscription on topic level.
Support get max consumers per subscription on topic level.
Support remove max consumers per subscription on topic level.

### Verifying this change
This change added tests and can be verified as follows:
- test set topic max consumers per subscription
- test remove topic max consumers per subscription
- test get topic topic max consumers per subscription
- test disabled topic max consumers per subscription
@Anonymitaet Anonymitaet added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. and removed doc-required Your PR changes impact docs and you will update later. labels Feb 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Your PR contains doc changes, no matter whether the changes are in markdown or code files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants