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 namespace policy for maxTopicsPerNamespace limit #9042

Merged

Conversation

hangc0276
Copy link
Contributor

Related to #8225

Changes

  1. Support namespace policy for maxTopicsPerNamespace limit
  2. Add the tests for this feature

@codelipenghui
Copy link
Contributor

@315157973 Could you please help review this PR?

Integer maxTopicsPerNamespace;
try {
maxTopicsPerNamespace = getNamespacePolicies(namespaceName).max_topics_per_namespace;
if (maxTopicsPerNamespace == null || maxTopicsPerNamespace < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When setting the value, it must be greater than or equal to 0, so there should be no case of <0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, maxTopicsPerNamespace is get from namespace policy, it must be greater than 0, i update the code.

validateNamespacePolicyOperation(namespaceName, PolicyName.MAX_TOPICS, PolicyOperation.WRITE);
validatePoliciesReadOnlyAccess();

if (maxTopicsPerNamespace != null && maxTopicsPerNamespace < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When setting the value, it must be greater than or equal to 0, so there should be no case of <0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The maxTopicsPerNamespace is passed by user, we should forbid user set the value < 0, so the check must be added.

admin.namespaces().createNamespace(namespace, Sets.newHashSet("use"));
admin.namespaces().setMaxTopicsPerNamespace(namespace, 10);

pulsarClient.newProducer().topic(topic + "1").create();
Copy link
Contributor

Choose a reason for hiding this comment

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

Will there be a problem if the created producer is not closed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, i will close them.

@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

.map(p -> p.max_topics_per_namespace)
.orElse(null);

if (maxTopicsPerNamespace == null || maxTopicsPerNamespace < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will there be no <0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remove it.

@hangc0276
Copy link
Contributor Author

/pulsarbot run-failure-checks

1 similar comment
@315157973
Copy link
Contributor

/pulsarbot run-failure-checks

@sijie sijie merged commit 2442071 into apache:master Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants