[pulsar-broker] add uniform load shedder strategy to distribute traffic uniformly across brokers #12902

rdhabalia · 2021-11-20T06:03:44Z

Motivation

Pulsar Broker provides a configurable way to configure various load shedding strategies based on usecases. It already has strategies to load balance based on broker resource utilization (ThresholdShedder) or based on bundle usage threshold (OverloadShedder).
We need another strategy to distribute load uniformly across all brokers. So, we are adding UniformLoadShedder strategy to achieve goal for uniform load in cluster.

NOTE

UniformLoadShedder also requires isolation and broker weight mechanism to also use strategy in heterogeneous hardware. I will create another PR for this enhancement.

github-actions · 2021-11-20T06:04:11Z

@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

github-actions · 2021-11-20T06:04:11Z

@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

…ic uniformly across brokers

eolivelli

+1
But I am not expert of this kind of algorithms.
So it is better to wait for some other review

codelipenghui · 2021-11-21T04:50:16Z

@hangc0276 Please help review the PR.

Jason918

And we need to update the doc of loadBalancerLoadSheddingStrategy about this new strategy.

Jason918 · 2021-11-21T08:19:46Z

conf/broker.conf

+# uniform load shedding. (eg: broker1 with 50K msgRate and broker2 with 30K msgRate
+# will have 66% msgRate difference and load balancer can unload bundles from broker-1
+# to broker-2)
+loadBalancerMsgRateDifferenceShedderThreshold=50


IMHO, It's better to add a notice about how to enable uniform load shedding, like set loadBalancerLoadSheddingStrategy="org.apache.pulsar.broker.loadbalance.impl.UniformLoadShedder"

added into documentation

lhotari · 2021-11-22T12:20:53Z

pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java

+                + "will have 4.5 times msgThroughout difference and load balancer can unload bundles "
+                + "from broker-1 to broker-2)"
+    )
+    private int loadBalancerMsgThroughputMultiplierDifferenceShedderThreshold = 4;


just wondering whether there would there need to be support for fractions like 1.5 ?

yes, value is double and it supports fractions.

Anonymitaet

@rdhabalia For the doc side, how about adding docs like below:

Add a section Load shedder strategy to load balance chapter
Explain 3 strategies (ThresholdShedder, OverloadShedder, and UniformLoadShedder), including "when to use" and "how to use" for each strategy

If so, can you add them? Thanks!

rdhabalia · 2021-11-24T20:27:57Z

@Anonymitaet I have added documentation, if needed then we can always add more documentations in future.

codelipenghui · 2021-11-25T02:37:31Z

I think we'd better use the batch rate instead of the msg rate? the batch rate consumes the broker CPU resource and throughput consumes the network bandwidth.

Anonymitaet

Hi @rdhabalia thanks for adding docs, but users may not have enough patience to wade through “chunks" of information, so I re-organize them here and left some comments.

Can you help review and answer the questions? If the content makes sense, you can implement it into your .md files, thanks.

rdhabalia · 2021-11-30T20:21:58Z

@codelipenghui

I think we'd better use the batch rate instead of the msg rate?

LoadBalancer only captures MsgRate and not batch msg, and that's because batch message is an abstraction for the client. Broker still considers batch message a single message in terms of processing and storage. batch-message size which something matters and that can be covered by msgThroughput threshold.

@Anonymitaet

Please don't block the PR due to the documentation format. yes, I agree, the better format helps but the reorganization of the content can be handled in separate PR as well because every individual has a different idea about the formation.

rdhabalia · 2021-11-30T20:25:36Z

@Anonymitaet fixed typo about loadBalancerMsgRateDifferenceShedderThreshold config based on your feedback in the doc

Anonymitaet · 2021-12-01T02:37:39Z

@Anonymitaet

Please don't block the PR due to the documentation format. yes, I agree, the better format helps but the reorganization of the content can be handled in separate PR as well because every individual has a different idea about the formation.

Hi @rdhabalia,

I did not mean to block anything, I just want to improve the user experience since no one would like to read "a mountain of" information at a glance. Technical information should be easy to find/understand/use.
Agree that documentation can be submitted in a separate PR.
Everyone has a different opinion on everything but there are some general rules/standards accepted by the majority. In technical writing, often, text that is difficult to understand in paragraph form becomes clear in a table format. Tables help highlight relationships among similar pieces of information and are easily scanned for quick reference.

rdhabalia · 2021-12-01T03:45:58Z

@Anonymitaet sure, I agree. let me create a separate PR with detailed info because we still have to add support of namespace isolation. so, can we unblock this PR and merge it for now and we can enhance with better user experience.

Anonymitaet · 2021-12-01T05:04:37Z

@rdhabalia yes, it makes sense. Separate doc PR is more convenient for cutting a release (cherry-pick changes) as well.

rdhabalia · 2021-12-01T05:08:46Z

@Anonymitaet I still see blocker for this PR. Please avoid creating blocker for DOC.

submit doc in another pr

Anonymitaet · 2021-12-01T08:13:50Z

@rdhabalia I label this PR with doc-required. Do not forget to revert doc changes in this PR and submit them in a separate PR, thanks.

…te traffic uniformly across brokers (apache#12902)" This reverts commit f965fb8.

…ic uniformly across brokers (apache#12902) * [pulsar-broker] add uniform load shedder strategy to distribute traffic uniformly across brokers * add documentation * fixed typo in documentation

rdhabalia added the area/broker label Nov 20, 2021

rdhabalia added this to the 2.10.0 milestone Nov 20, 2021

rdhabalia requested review from merlimat, sijie, hangc0276, jiazhai, eolivelli and codelipenghui November 20, 2021 06:03

rdhabalia self-assigned this Nov 20, 2021

github-actions bot added the doc-label-missing label Nov 20, 2021

rdhabalia force-pushed the uniform_shedder branch from d65500a to 4f9fcf8 Compare November 20, 2021 06:06

[pulsar-broker] add uniform load shedder strategy to distribute traff…

c0bae65

…ic uniformly across brokers

rdhabalia force-pushed the uniform_shedder branch from 4f9fcf8 to c0bae65 Compare November 20, 2021 06:09

rdhabalia mentioned this pull request Nov 20, 2021

Optimize ThresholdShedder strategy: the low-load Broker cannot be fully utilized #12471

Closed

eolivelli approved these changes Nov 20, 2021

View reviewed changes

Jason918 reviewed Nov 21, 2021

View reviewed changes

Anonymitaet added doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. and removed doc-label-missing labels Nov 22, 2021

lhotari reviewed Nov 22, 2021

View reviewed changes

Anonymitaet reviewed Nov 24, 2021

View reviewed changes

add documentation

297a9f6

rdhabalia force-pushed the uniform_shedder branch from 83daf22 to 297a9f6 Compare November 24, 2021 20:27

Anonymitaet previously requested changes Nov 25, 2021

View reviewed changes

fixed typo in documentation

6536a71

Anonymitaet added doc-required Your PR changes impact docs and you will update later. and removed doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. labels Dec 1, 2021

rdhabalia merged commit f965fb8 into apache:master Dec 1, 2021

rdhabalia mentioned this pull request Dec 2, 2021

[feat] [broker] support broker isolation for uniform load shedder strategy #13080

Open

lordcheng10 added a commit to lordcheng10/pulsar that referenced this pull request Dec 5, 2021

Revert "[pulsar-broker] add uniform load shedder strategy to distribu…

480355f

…te traffic uniformly across brokers (apache#12902)" This reverts commit f965fb8.

lordcheng10 mentioned this pull request Dec 5, 2021

When the traffic to be unloaded is too small, it means that maxThroug… #13140

Closed

1 task

Anonymitaet added doc-complete Your PR changes impact docs and the related docs have been already added. and removed doc-required Your PR changes impact docs and you will update later. labels Dec 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pulsar-broker] add uniform load shedder strategy to distribute traffic uniformly across brokers #12902

[pulsar-broker] add uniform load shedder strategy to distribute traffic uniformly across brokers #12902

rdhabalia commented Nov 20, 2021

github-actions bot commented Nov 20, 2021

github-actions bot commented Nov 20, 2021

eolivelli left a comment

codelipenghui commented Nov 21, 2021

Jason918 left a comment

Jason918 Nov 21, 2021

rdhabalia Nov 24, 2021

lhotari Nov 22, 2021

rdhabalia Nov 24, 2021

Anonymitaet left a comment

rdhabalia commented Nov 24, 2021

codelipenghui commented Nov 25, 2021

Anonymitaet left a comment

rdhabalia commented Nov 30, 2021

rdhabalia commented Nov 30, 2021

Anonymitaet commented Dec 1, 2021

rdhabalia commented Dec 1, 2021

Anonymitaet commented Dec 1, 2021

rdhabalia commented Dec 1, 2021

Anonymitaet commented Dec 1, 2021

[pulsar-broker] add uniform load shedder strategy to distribute traffic uniformly across brokers #12902

[pulsar-broker] add uniform load shedder strategy to distribute traffic uniformly across brokers #12902

Conversation

rdhabalia commented Nov 20, 2021

Motivation

NOTE

github-actions bot commented Nov 20, 2021

github-actions bot commented Nov 20, 2021

eolivelli left a comment

Choose a reason for hiding this comment

codelipenghui commented Nov 21, 2021

Jason918 left a comment

Choose a reason for hiding this comment

Jason918 Nov 21, 2021

Choose a reason for hiding this comment

rdhabalia Nov 24, 2021

Choose a reason for hiding this comment

lhotari Nov 22, 2021

Choose a reason for hiding this comment

rdhabalia Nov 24, 2021

Choose a reason for hiding this comment

Anonymitaet left a comment

Choose a reason for hiding this comment

rdhabalia commented Nov 24, 2021

codelipenghui commented Nov 25, 2021

Anonymitaet left a comment

Choose a reason for hiding this comment

rdhabalia commented Nov 30, 2021

rdhabalia commented Nov 30, 2021

Anonymitaet commented Dec 1, 2021

rdhabalia commented Dec 1, 2021

Anonymitaet commented Dec 1, 2021

rdhabalia commented Dec 1, 2021

Anonymitaet commented Dec 1, 2021