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

[Doc] Add doc on how to configure max subscriptions per topic at the topic level #9748

Merged
merged 5 commits into from
Mar 5, 2021

Conversation

Jennifer88huang-zz
Copy link
Contributor

Master Issue: #8866
Related PR: #8948

Motivation

In #8948, we support configuring max subscriptions at topic level.

Modifications

  1. Add related doc content for this feature.
  2. Fix some minor typos.

@Jennifer88huang-zz
Copy link
Contributor Author

@315157973 we might need to add related options for set-max-subscriptions if we have. Could you provide available options? Thank you.

@Jennifer88huang-zz Jennifer88huang-zz self-assigned this Feb 27, 2021
@Jennifer88huang-zz Jennifer88huang-zz added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. component/topic-policy release/2.7.1 labels Feb 27, 2021
@Jennifer88huang-zz Jennifer88huang-zz added this to the 2.8.0 milestone Feb 27, 2021

Usage
```bash
$ pulsar-admin topics set-max-subscriptions tenant/namespace/topic options
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks good, but I have some questions:
What does tenant/namespace/topic mean?

If we use this command,a topic name should be passed in here, such as:
persistent://tenant/namespace/topic

The entire command looks like this:
pulsar-admin topics set-max-subscriptions [topic name] [options]

@315157973
Copy link
Contributor

Now this command is also called get-max-subscriptions-per-topic at topic-level. But I think it would be more reasonable to call get-max-subscriptions. I will post a PR to fix this problem later.
Because, we have get-max-producers-per-topic at namespace-level, but it is called get-max-producers at topic-level. We should be consistent with these rules.

@315157973
Copy link
Contributor

Here is the PR #9750

Comment on lines +1855 to +1857
* `set-max-subscriptions`
* `get-max-subscriptions`
* `remove-max-subscriptions`
Copy link
Member

Choose a reason for hiding this comment

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

Need to add them to pulsar-admin website? or is set-max-subscriptions same to set-max-subscriptions-per-topic? Same comments for the other two commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

set-max-subscriptions is the same to set-max-subscriptions-per-topic, fixed in PR #9750

@Jennifer88huang-zz
Copy link
Contributor Author

@315157973 @Anonymitaet updated, PTAL again, thank you.

@Jennifer88huang-zz
Copy link
Contributor Author

@315157973 @codelipenghui if there is no further issue, could you approve and merge it? If you have any concern, feel free to comment. Thank you.

@Jennifer88huang-zz Jennifer88huang-zz merged commit d05b04c into apache:master Mar 5, 2021
mlyahmed pushed a commit to mlyahmed/pulsar that referenced this pull request Mar 5, 2021
…topic level (apache#9748)

* add set-max-subscriptions for topic level

* update

* add set-max-subscription for topic level

* update

* update
@Jennifer88huang-zz Jennifer88huang-zz deleted the 8948t branch March 8, 2021 01:26
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. release/2.7.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants