Skip to content

Conversation

@Nicklee007
Copy link
Contributor

@Nicklee007 Nicklee007 commented Jul 25, 2022

Motivation

As the brokerAvgResourceUsageWithWeight stored the broker's history resource usage, if a broker is high load before restart, after the broker restart always show a high load history resource usage, and loadBalancerHistoryResourcePercentage = 0.9, which will cause the first few times the broker can not be select as best broker to load bundles. Only when selectBrokerForAssignment invoke few time's , the brokerAvgResourceUsageWithWeight could be neutralized by the current resourceUsage.
But bundles unload is not a frequently operator if not disable doLoadShedding,it will wait a long time; So if a broker restarted we can clean the brokers history resource usage to guaranty the no bundles or no msgRate broker can be select as the one of best brokers.

historyUsage = historyUsage == null
? resourceUsage : historyUsage * historyPercentage + (1 - historyPercentage) * resourceUsage;

Modifications

  1. If the broker not own bundles or the msgRate is 0, use current resourceUsage to cover the historyUsage.
  2. add some unit test.

Documentation

  • doc-not-needed
    (Please explain why)

Matching PR in forked repository
PR in forked repository:Nicklee007#3

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jul 25, 2022
@Nicklee007 Nicklee007 force-pushed the improve-historyusage-clean-for-restart-broker branch from e386176 to 8cf80d6 Compare August 2, 2022 03:12
@Nicklee007 Nicklee007 force-pushed the improve-historyusage-clean-for-restart-broker branch 4 times, most recently from bd24ac3 to 3a4ae32 Compare August 26, 2022 08:32
@Nicklee007 Nicklee007 closed this Aug 26, 2022
@Nicklee007 Nicklee007 reopened this Aug 26, 2022
@Nicklee007 Nicklee007 force-pushed the improve-historyusage-clean-for-restart-broker branch 4 times, most recently from b5f8001 to cb48cba Compare September 5, 2022 08:24
@Nicklee007 Nicklee007 force-pushed the improve-historyusage-clean-for-restart-broker branch from cb48cba to c83b387 Compare September 21, 2022 09:16
@github-actions
Copy link

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions bot added the Stale label Oct 22, 2022
@Nicklee007 Nicklee007 force-pushed the improve-historyusage-clean-for-restart-broker branch from c83b387 to 3c0c603 Compare December 7, 2022 12:59
@Nicklee007
Copy link
Contributor Author

/pulsarbot run-failure-checks

Copy link
Contributor

@HQebupt HQebupt left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

codecov-commenter commented Dec 8, 2022

Codecov Report

Merging #16766 (3c0c603) into master (99e26f5) will increase coverage by 0.41%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #16766      +/-   ##
============================================
+ Coverage     44.79%   45.20%   +0.41%     
- Complexity    10026    10817     +791     
============================================
  Files           703      763      +60     
  Lines         68808    73575    +4767     
  Branches       7375     7912     +537     
============================================
+ Hits          30822    33260    +2438     
- Misses        34378    36592    +2214     
- Partials       3608     3723     +115     
Flag Coverage Δ
unittests 45.20% <0.00%> (+0.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...loadbalance/impl/LeastResourceUsageWithWeight.java 0.00% <0.00%> (ø)
...pache/bookkeeper/mledger/impl/RangeSetWrapper.java 43.39% <0.00%> (-2.61%) ⬇️
...org/apache/bookkeeper/mledger/util/RangeCache.java 67.04% <0.00%> (-2.28%) ⬇️
...he/pulsar/broker/admin/v2/NonPersistentTopics.java 60.64% <0.00%> (-1.39%) ⬇️
...rg/apache/pulsar/client/impl/PulsarClientImpl.java 43.85% <0.00%> (-0.95%) ⬇️
...apache/pulsar/client/impl/AutoClusterFailover.java 69.44% <0.00%> (-0.56%) ⬇️
...he/pulsar/client/impl/MultiTopicsConsumerImpl.java 22.86% <0.00%> (-0.12%) ⬇️
...pulsar/metadata/impl/stats/MetadataStoreStats.java 88.88% <0.00%> (ø)
...g/apache/pulsar/metadata/api/NotificationType.java 100.00% <0.00%> (ø)
...ache/pulsar/metadata/api/MetadataStoreFactory.java 0.00% <0.00%> (ø)
... and 154 more

@github-actions github-actions bot removed the Stale label Dec 9, 2022
@gaozhangmin gaozhangmin merged commit 66f8f8c into apache:master Dec 9, 2022
Demogorgon314 pushed a commit to Demogorgon314/pulsar that referenced this pull request Dec 26, 2022
…of the best brokers when selectBroker in LeastResourceUsageWithWeight (apache#16766)

Co-authored-by: nicklixinyang <nicklixinyang@didiglobal.com>
Demogorgon314 pushed a commit to Demogorgon314/pulsar that referenced this pull request Dec 29, 2022
…of the best brokers when selectBroker in LeastResourceUsageWithWeight (apache#16766)

Co-authored-by: nicklixinyang <nicklixinyang@didiglobal.com>
lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Jan 10, 2023
…of the best brokers when selectBroker in LeastResourceUsageWithWeight (apache#16766)

Co-authored-by: nicklixinyang <nicklixinyang@didiglobal.com>
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 ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants