Skip to content

[improve][broker]improve the getAntiAffinityNamespaceOwnedBrokers check exclude the current namespace cause count add. - #16563

Open
Nicklee007 wants to merge 1 commit into
apache:masterfrom
Nicklee007:improve-anti-affinity-owned-brokers-check
Open

[improve][broker]improve the getAntiAffinityNamespaceOwnedBrokers check exclude the current namespace cause count add.#16563
Nicklee007 wants to merge 1 commit into
apache:masterfrom
Nicklee007:improve-anti-affinity-owned-brokers-check

Conversation

@Nicklee007

@Nicklee007 Nicklee007 commented Jul 13, 2022

Copy link
Copy Markdown
Contributor

Motivation

As the PIP 7: Pulsar Failure domain and Anti affinity namespaces design, the anti affinity namespaces should be distributed to evenly across all domain and all the brokers.
But in method getAntiAffinityNamespaceOwnedBrokers , brokerToAntiAffinityNamespaceCount add a count even the namespace equal the given bundle's namespace which will be load. The behavior will cause the namespace easy to distributed to
a broker which has another namespace in anti affinity group, the behavior broke the anti affinity balance. It's better behavior is the same namespace should be distributed to those broker which has loaded the same namespace when all broker have load at least one namespace in ti affinity group.

there is some case
ns-0 ns-1 ns-2 are all set the same anti affinity group like 'a-group

broker-0 own ns-0 bundle-0;
broker-1 own ns-1 bundle-0;
broker-2 own ns-2 bundle-0;
then another ns-2 bundle-1 need choice a broker to load. As the old policy, broker-0 broker-1 and broker-2 are all satisfy the least NamespaceCount; but if ns-2 bundle-1 load to broker-0 or broker-1 will broke the anti affinity balance. ns-2 bundle-1 need be load by broker-2 is better.

Also, the behavior will cause give up doLoadShedding when the all broker own one namespace in anti affinity group, but the different broker owned namespace bundle count and payload is different.
So brokerToAntiAffinityNamespaceCount should exclude the given namespace count add.

Modifications

  1. In LoadManagerShared.class getAntiAffinityNamespaceOwnedBrokers method, exclude the given namespace count add.
  2. changed shouldAntiAffinityNamespaceUnload check;
  3. add some unit test.

Documentation

  • doc-not-needed

Matching PR in forked repository

PR in forked repository: Nicklee007#5

@Nicklee007
Nicklee007 force-pushed the improve-anti-affinity-owned-brokers-check branch 3 times, most recently from 8ee2884 to 668d99e Compare July 20, 2022 03:00
@Nicklee007

Copy link
Copy Markdown
Contributor Author

@codelipenghui @Jason918 @eolivelli Could you help to review this PR, Thx.

@Jason918

Copy link
Copy Markdown
Contributor

there is some case
broker-0 own ns-0 bundle-0;
broker-1 own ns-1 bundle-0;
broker-2 own ns-2 bundle-0;
then another ns-2 bundle-1 need choice a broker to load. As the old policy, broker-0 broker-1 and broker-2 are all satisfy the least NamespaceCount; but if ns-2 bundle-1 load to broker-0 or broker-1 will broke the anti affinity balance. ns-2 bundle-1 need be load by broker-2 is better.

What's the anti affinity setting in this case?

@Nicklee007

Nicklee007 commented Jul 28, 2022

Copy link
Copy Markdown
Contributor Author

What's the anti affinity setting in this case?

@Jason918 ns-0 ns-1 ns-2 are all set the same anti affinity group like 'a-group', and the three ns be anti affinity load is expected.

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Jul 28, 2022
@Nicklee007

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@Nicklee007
Nicklee007 force-pushed the improve-anti-affinity-owned-brokers-check branch from 4a3e4e8 to 197faef Compare September 21, 2022 09:17
@github-actions

Copy link
Copy Markdown

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-anti-affinity-owned-brokers-check branch 4 times, most recently from c29ea60 to 83e5536 Compare December 8, 2022 12:35
@Nicklee007

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@codecov-commenter

codecov-commenter commented Jan 11, 2023

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 34.68%. Comparing base (b05fddb) to head (118b16b).
⚠️ Report is 3021 commits behind head on master.

Files with missing lines Patch % Lines
...sar/broker/loadbalance/impl/LoadManagerShared.java 0.00% 5 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (b05fddb) and HEAD (118b16b). Click for more details.

HEAD has 3 uploads less than BASE
Flag BASE (b05fddb) HEAD (118b16b)
unittests 8 5
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #16563       +/-   ##
=============================================
- Coverage     45.64%   34.68%   -10.96%     
+ Complexity    11043     6738     -4305     
=============================================
  Files           773      633      -140     
  Lines         74463    59892    -14571     
  Branches       8018     6241     -1777     
=============================================
- Hits          33986    20775    -13211     
+ Misses        36687    36320      -367     
+ Partials       3790     2797      -993     
Flag Coverage Δ
unittests 34.68% <0.00%> (-10.96%) ⬇️

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

Files with missing lines Coverage Δ
...sar/broker/loadbalance/impl/LoadManagerShared.java 39.39% <0.00%> (-4.03%) ⬇️

... and 286 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Nicklee007

Copy link
Copy Markdown
Contributor Author

/pulsarbot run-failure-checks

@github-actions

Copy link
Copy Markdown

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

@github-actions github-actions Bot added the Stale label Feb 11, 2023
@poorbarcode poorbarcode modified the milestones: 3.0.0, 3.1.0 Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants