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

Add rate limit support for Replicator between clusters #4273

Merged
merged 4 commits into from
May 20, 2019

Conversation

jiazhai
Copy link
Member

@jiazhai jiazhai commented May 14, 2019

Currently the rate limit between replication clusters is not able to control.
In Geo-replication, once a cluster is offline, and after a long time, if it comes back, it may get a lot of messages from other clusters, and may use all of the network bandwidth.
This PR tries to provided a way to control the rate limit between cluster replications. Add rate limit support for Replicator between clusters.

changes:

  • change DispatchRateLimiter.java to support 3 kind type: Topic, subscription, replicator.
  • add DispatchRateLimiter support in PersistentReplicator.
  • add test and docs

@jiazhai jiazhai added this to the 2.4.0 milestone May 14, 2019
@jiazhai jiazhai self-assigned this May 14, 2019
@jiazhai
Copy link
Member Author

jiazhai commented May 15, 2019

run integration tests
run java8 tests

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.

Overall looks good to me, just some simple ideas that I want to discuss together.
Considering that we have many features need dispatch rate limit in the future and limiter can effect different levels

Features:

  • Consumption
  • Reader
  • Replicator

Effect levels

  • Tenant
  • Namespace
  • Topic
  • Subscription(Consumption only)

Difference for all scenarios is the definition of DispatchRate. This will correspond to the configuration of different brokers and policies.
Leaving the common part in DispatchRateLimiter and allow users to specify the configuration and polices will be more scalable.

@jiazhai
Copy link
Member Author

jiazhai commented May 16, 2019

@codelipenghui thanks for the comments, It is a good idea, we may need consider more for DispatchRateLimiter in the future. Would you please open an issue to track your idea there?

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

The change LGTM. Just very minor comment.

@rdhabalia Please take a look as well.

@sijie sijie added area/broker type/feature The PR added a new feature or issue requested a new feature labels May 17, 2019
@sijie
Copy link
Member

sijie commented May 18, 2019

ping @rdhabalia

@sijie sijie merged commit 84d02ac into apache:master May 20, 2019
sijie pushed a commit that referenced this pull request Jan 18, 2020
### Motivation
Some parameters are added in the `broker.conf` and `standalone.conf` files. However, those parameters are not updated in the docs.
See the following PRs for details: #4150, #4066, #4197, #3819, #4261, #4273, #4320.

### Modifications
Add those parameter info, and sync docs with the code.

Does not update the description quite much, there are two reasons for this:
1. Keep doc content consistent with code. We need to update the description for those parameters in the code first, and then sync them in docs.
2. Will adopt a generator to generate those content automatically in the near future.
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
### Motivation
Some parameters are added in the `broker.conf` and `standalone.conf` files. However, those parameters are not updated in the docs.
See the following PRs for details: apache#4150, apache#4066, apache#4197, apache#3819, apache#4261, apache#4273, apache#4320.

### Modifications
Add those parameter info, and sync docs with the code.

Does not update the description quite much, there are two reasons for this:
1. Keep doc content consistent with code. We need to update the description for those parameters in the code first, and then sync them in docs.
2. Will adopt a generator to generate those content automatically in the near future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/broker type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants