[Improve][broker] When calculating resource usage, use getMaxResourceUsageWithWeight instead of getMaxResourceUsage

[lordcheng10]

Motivation

Use getMaxResourceUsageWithWeight instead of getMaxResourceUsage in selectBrokerForAssignment.

Documentation

Check the box below or label this PR directly.

Need to update docs?

• [ ] doc-required (Your PR needs to update docs and you will update later)

• [x] doc-not-needed (Please explain why)

• [ ] doc (Your PR contains doc changes)

• [ ] doc-complete (Docs have been already added)

[lordcheng10]

@eolivelli @hanbo1990 @Shoothzj PTAL,thanks!

[lordcheng10]

@codelipenghui PTAL,Thanks!

[lordcheng10]

@gaozhangmin PTAL,thanks!

[Jason918]

Is this the same with #16281 ?

[heesung-sohn]

Is this the same with #16281 ?

Afaik, https://github.com/apache/pulsar/pull/16281 is adding a new strategy, whereas this PR is updating the current default one LeastLongTermMessageRate.java

[heesung-sohn]

To me this fix makes sense. However, I am wondering if there was any reason why it uses getMaxResourceUsage() instead of getMaxResourceUsageWithWeight().

The impact of this change could be broad(hopefully in a positive way) for the clusters that configure the resource weights differently.

Nevertheless, LGTM.

[HQebupt]

To me this fix makes sense. However, I am wondering if there was any reason why it uses getMaxResourceUsage() instead of getMaxResourceUsageWithWeight().

The impact of this change could be broad(hopefully in a positive way) for the clusters that configure the resource weights differently.

Nevertheless, LGTM.

I agree. The resource usage weight configurations only work in ThresholdShedder strategy currently. Why use getMaxResourceUsage() instead of getMaxResourceUsageWithWeight() in LeastLongTermMessageRate？

[lordcheng10]

Both getMaxResourceUsage() and getMaxResourceUsageWithWeight() are only the calculation methods of resource usage, and getMaxResourceUsageWithWeight() is more flexible than getMaxResourceUsage(), so whether it is OverloadShedder or ThresholdShedder, getMaxResourceUsageWithWeight() is more suitable.

I think getMaxResourceUsage() should be discarded, and getMaxResourceUsageWithWeight() should be used uniformly for resource calculation, not only in selectBrokerForAssignment() method.

@HQebupt @heesung-sn @eolivelli @Jason918

[Jason918]

IMO, the real problem is the definition of broker load (aka resource usage) is not consistent in bundle shedder and bundle assignment. The load balance module should have a better abstraction that contains both shedding strategy and loading strategy for bundles, so that it can perform a clear balancing target, which can be message rate or cpu load or an abstract resource usage like MaxResourceUsageWithWeight.

@lordcheng10 @heesung-sn @HQebupt @eolivelli

[github-actions[bot]]

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

[keyboardbobo]

@lordcheng10 @heesung-sn @HQebupt @eolivelli @Jason918 Is there any progress on this PR? If different dimensions are considered when bundle unloading and assigning, there may be frequent balancing leading to large traffic fluctuations. Moreover, the assigned broker has just been unloaded, which will cause unnecessary traffic fluctuations.