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

[fix][broker] fix UniformLoadShedder selecet wrong overloadbroker and underloadbroker #21025

Merged
merged 2 commits into from
Sep 12, 2023

Conversation

aloyszhang
Copy link
Contributor

@aloyszhang aloyszhang commented Aug 18, 2023

Fixes #21016

Motivation

fix UniformLoadShedder select the wrong overloadbroker and underloadbroker

Modifications

  1. correct the procedure of selecting overload and underload broker
  2. separating msgRate and msgThrought, and msgRate has higher priority(same as before)

Documentation

  • doc-not-needed

Matching PR in forked repository

PR in forked repository: aloyszhang#19

@aloyszhang
Copy link
Contributor Author

aloyszhang commented Aug 18, 2023

Update real workload result.
image
Pulsar clusters with:

  • 3 brokers

  • 6 topics, each one has 11 partitions, topics have different msgRates and msgSizes

At 16:52 start load-balance with this fix and the following parameters:

  • loadBalancerMsgThroughputMultiplierDifferenceShedderThreshold=1.3
  • loadBalancerMsgRateDifferenceShedderThreshold=0 (disable)

We can see all brokers are balanced well based on throughput.
At 17:04 modify loadBalancerMsgThroughputMultiplierDifferenceShedderThreshold from 1.3 to 1.05
All brokers load are more balanced.

Then fallback server to origin UniformLoadShedder(without this fix) and restart broker at 17:15, all configurations are not modified:

  • loadBalancerMsgThroughputMultiplierDifferenceShedderThreshold=1.05
  • loadBalancerMsgRateDifferenceShedderThreshold=0 (disable)

Brokers load balanced at 17:24, but it's not quite balanced, maxThroughput is 128MB/s and minThroughput is 88 MB/s, the finnaly loadDifferent is 128/88 = 1.45 which is bigger than loadBalancerMsgThroughputMultiplierDifferenceShedderThreshold(1.05)

Copy link
Contributor

@lordcheng10 lordcheng10 left a comment

Choose a reason for hiding this comment

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

LGTM

@BewareMyPower BewareMyPower merged commit fc86f3b into apache:master Sep 12, 2023
45 checks passed
@frankjkelly
Copy link
Contributor

This is great - does it need to be backported to prior versions?

@aloyszhang
Copy link
Contributor Author

This is great - does it need to be backported to prior versions?

@frankjkelly I'll backport this change to versions after 2.10, including 2.10, 2.11, 3.0 and 3.1

aloyszhang added a commit to aloyszhang/pulsar that referenced this pull request Sep 14, 2023
aloyszhang added a commit to aloyszhang/pulsar that referenced this pull request Sep 14, 2023
aloyszhang added a commit to aloyszhang/pulsar that referenced this pull request Sep 14, 2023
@aloyszhang
Copy link
Contributor Author

Backport pull request links:
branch-3.1 : #21180
branch-3.0: #21179
branch-2.11: #21178
branch-2.10: #21182

@frankjkelly
Copy link
Contributor

This is great - does it need to be backported to prior versions?

@frankjkelly I'll backport this change to versions after 2.10, including 2.10, 2.11, 3.0 and 3.1

That would be wonderful - I am using ThresholdShedder in 2.10.3 and am not thrilled with how complex it is to manage. I'd love to use UniformShedder in 2.11.X with this fix. Thanks so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] UniformLoadShedder selects wrong overloadbroker and underloadbroker
8 participants