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 13756][Broker]Optimize topic policy with HierarchyTopicPolicies about publishRate #14267

Merged
merged 3 commits into from
Feb 16, 2022

Conversation

AnonHxy
Copy link
Contributor

@AnonHxy AnonHxy commented Feb 14, 2022

Master Issue: #13756

Motivation

See issue: #13756

Modifications

  • Add field publishRate in org.apache.pulsar.common.policies.data.HierarchyTopicPolicies
  • Methods to update publishRate in org.apache.pulsar.broker.service.AbstractTopic

Verifying this change

  • Make sure that the change passes the CI checks.

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

  • no-need-doc

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 14, 2022
topicPolicies.getDelayedDeliveryEnabled().updateTopicValue(data.getDelayedDeliveryEnabled());
topicPolicies.getDelayedDeliveryTickTimeMillis().updateTopicValue(data.getDelayedDeliveryTickTimeMillis());
topicPolicies.getCompactionThreshold().updateTopicValue(data.getCompactionThreshold());
}

private PublishRate normalize(PublishRate publishRate) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to move this to a static method in PublishRate class.

topicPolicies.getDelayedDeliveryEnabled().updateTopicValue(data.getDelayedDeliveryEnabled());
topicPolicies.getDelayedDeliveryTickTimeMillis().updateTopicValue(data.getDelayedDeliveryTickTimeMillis());
topicPolicies.getCompactionThreshold().updateTopicValue(data.getCompactionThreshold());
}

private PublishRate normalize(PublishRate publishRate) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to move this to a static method in PublishRate class.

@AnonHxy AnonHxy changed the title [Issue 13756][Broker]Optimize topic policy with HierarchyTopicPolicies about publishRate [WIP][Issue 13756][Broker]Optimize topic policy with HierarchyTopicPolicies about publishRate Feb 15, 2022
@@ -1061,7 +1044,8 @@ protected boolean isExceedMaximumMessageSize(int size, PublishContext publishCon
/**
* update topic publish dispatcher for this topic.
*/
protected void updatePublishDispatcher(PublishRate publishRate) {
public void updatePublishDispatcher() {
PublishRate publishRate = topicPolicies.getPublishRate().get();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this return null?

Copy link
Contributor Author

@AnonHxy AnonHxy Feb 15, 2022

Choose a reason for hiding this comment

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

It can not return null, I will remove the if (publishRate != null below @eolivelli

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eolivelli PTAL

@AnonHxy AnonHxy changed the title [WIP][Issue 13756][Broker]Optimize topic policy with HierarchyTopicPolicies about publishRate [Issue 13756][Broker]Optimize topic policy with HierarchyTopicPolicies about publishRate Feb 15, 2022
@codelipenghui codelipenghui added this to the 2.10.0 milestone Feb 16, 2022
@codelipenghui codelipenghui merged commit b87aa79 into apache:master Feb 16, 2022
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants