pulsar icon indicating copy to clipboard operation
pulsar copied to clipboard

[improve] [pip] PIP-364: Introduce a new load balance algorithm AvgShedder

Open thetumbled opened this issue 1 year ago • 2 comments

Motivation

Implementation PR for PIP: https://github.com/apache/pulsar/pull/22946.

Modifications

  • Introduce new algo: AvgShedder.
  • provide a way to bind shedding strategy and placement strategy.

Verifying this change

  • [ ] Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • [ ] Dependencies (add or upgrade a dependency)
  • [ ] The public API
  • [ ] The schema
  • [ ] The default values of configurations
  • [ ] The threading model
  • [ ] The binary protocol
  • [ ] The REST endpoints
  • [ ] The admin CLI options
  • [ ] The metrics
  • [ ] Anything that affects deployment

Documentation

  • [ ] doc
  • [x] doc-required
  • [ ] doc-not-needed
  • [ ] doc-complete

Matching PR in forked repository

PR in forked repository: https://github.com/thetumbled/pulsar/pull/59

thetumbled avatar Jun 20 '24 06:06 thetumbled

@thetumbled Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

github-actions[bot] avatar Jun 20 '24 06:06 github-actions[bot]

As the proposal PR has been merged, we can proceed with the implementation PR, thanks. @lhotari @heesung-sn @Demogorgon314 @BewareMyPower @poorbarcode

thetumbled avatar Jun 28 '24 10:06 thetumbled

Besides, the findBundlesForUnloading method is too long (about 200 lines), which is hard to read. It's worth splitting it into multiple private methods. From what I see, there should be 3 steps:

  1. loadData.getBrokerData() -> updated brokerScoreMap (Map<String, Double>) and its sorted keys according to the value comparation
  2. Update brokerHitCountForLow and brokerHitCountForHigh according to the threshold related configs.
  3. Update and return selectedBundlesCache according to loadData.getRecentlyUnloadedBundles() and the two maps above.

Updated, thanks for review!

thetumbled avatar Jul 09 '24 02:07 thetumbled

Good job! Can you add the AvgShedder algorithm for ExtensibleLoadManager.

Sounds good, i will try to acheive that.

thetumbled avatar Jul 11 '24 07:07 thetumbled

PIP-364: Introduce a new load balance algorithm AvgShedder

thetumbled avatar Jul 11 '24 09:07 thetumbled

Hi @thetumbled, can you help send a discussion to cherry-pick this new algorithm into branch-3.0, branch-3.2, and branch-3.3?

Demogorgon314 avatar Jul 16 '24 09:07 Demogorgon314

Hi @thetumbled, can you help send a discussion to cherry-pick this new algorithm into branch-3.0, branch-3.2, and branch-3.3?

discussion link: https://lists.apache.org/thread/9yqq8wky1k73hqz6xhvstk6x0ycknqgt

thetumbled avatar Jul 16 '24 09:07 thetumbled

Tip: If you want to cherry pick this pr, please cherry pick this patch too: https://github.com/apache/pulsar/pull/23156

thetumbled avatar Aug 13 '24 04:08 thetumbled