Skip to content

Conversation

@Nicklee007
Copy link
Contributor

Motivation

Avoid the bestBroker choice broker exceed candidates when selectBroker in LeastResourceUsageWithWeight.
When loadManager leader running a period of time, brokerAvgResourceUsageWithWeight will contain almost all brokers, the judge will cause we selected broker exceed candidates, it's out of expectation.

Modifications

  1. check if candidates contain the broker before add the broker to bestBrokers.

Documentation

  • doc-not-needed
    (Please explain why)

Copy link
Member

@shibd shibd left a comment

Choose a reason for hiding this comment

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

Nice catch! Could you add a unit test to protect this change?

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jul 22, 2022
@codelipenghui codelipenghui added this to the 2.11.0 milestone Jul 23, 2022
@codelipenghui codelipenghui added release/2.10.2 release/2.9.4 type/bug The PR fixed a bug or issue reported a bug area/broker labels Jul 23, 2022
@codelipenghui
Copy link
Contributor

Nice catch! Could you add a unit test to protect this change?

+1 Please add test to avoid regression.

@HQebupt
Copy link
Contributor

HQebupt commented Jul 23, 2022

Nice catch. +1

@Nicklee007
Copy link
Contributor Author

Nice catch! Could you add a unit test to protect this change?

@codelipenghui @shibd add unit test,PTAL~

Copy link
Member

@Demogorgon314 Demogorgon314 left a comment

Choose a reason for hiding this comment

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

Nice catch! LGTM.

@Nicklee007 Nicklee007 force-pushed the fix-selectBroker-out-of-candidates-in-LeastResourceUsageWithWeight branch from f730f3f to fb9762b Compare July 25, 2022 06:37
@codelipenghui codelipenghui modified the milestones: 2.11.0, 2.12.0 Jul 26, 2022
@Nicklee007 Nicklee007 force-pushed the fix-selectBroker-out-of-candidates-in-LeastResourceUsageWithWeight branch from fb9762b to e6d53fa Compare July 29, 2022 06:58
@Nicklee007
Copy link
Contributor Author

/pulsarbot run-failure-checks

@gaozhangmin gaozhangmin merged commit 0c1b3de into apache:master Aug 1, 2022
Gleiphir2769 pushed a commit to Gleiphir2769/pulsar that referenced this pull request Aug 4, 2022
…n selectBroker in LeastResourceUsageWithWeight (apache#16743)

* avoid the bestBroker exceed candidates when selectBroker

* add some unit test.

* improve the restart broker can load bundle as one of the best brokers

Co-authored-by: nicklixinyang <nicklixinyang@didiglobal.com>
@codelipenghui codelipenghui modified the milestones: 2.12.0, 2.11.0 Aug 4, 2022
@codelipenghui
Copy link
Contributor

Removed the release/2.10.2 and release/2.9.4 labels since we don't have LeastResourceUsageWithWeight on branch-2.10 and branch-2.9

dragonls pushed a commit to dragonls/pulsar that referenced this pull request Oct 21, 2022
…n selectBroker in LeastResourceUsageWithWeight (apache#16743)

* avoid the bestBroker exceed candidates when selectBroker

* add some unit test.

* improve the restart broker can load bundle as one of the best brokers

Co-authored-by: nicklixinyang <nicklixinyang@didiglobal.com>

(cherry picked from commit 0c1b3de)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker doc-not-needed Your PR changes do not impact docs type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants