You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/07/01 05:43:29 UTC

[GitHub] [pulsar] lordcheng10 opened a new pull request, #16319: use getMaxResourceUsageWithWeight in selectBrokerForAssignment

lordcheng10 opened a new pull request, #16319:
URL: https://github.com/apache/pulsar/pull/16319

   ### Motivation
   use getMaxResourceUsageWithWeight 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)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] lordcheng10 commented on pull request #16319: [Improve][broker] Use getMaxResourceUsageWithWeight instead of getMaxResourceUsage in selectBrokerForAssignment

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1171962533

   @gaozhangmin PTAL,thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] Jason918 commented on pull request #16319: [Improve][broker] Use getMaxResourceUsageWithWeight instead of getMaxResourceUsage in selectBrokerForAssignment

Posted by GitBox <gi...@apache.org>.
Jason918 commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1171976745

   Is this the same with #16281 ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] lordcheng10 commented on pull request #16319: [Improve][broker] Use getMaxResourceUsageWithWeight instead of getMaxResourceUsage in selectBrokerForAssignment

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1171962188

   @codelipenghui PTAL,Thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] heesung-sn commented on pull request #16319: [Improve][broker] Use getMaxResourceUsageWithWeight instead of getMaxResourceUsage in selectBrokerForAssignment

Posted by GitBox <gi...@apache.org>.
heesung-sn commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1172695290

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [Improve][broker] When calculating resource usage, use getMaxResourceUsageWithWeight instead of getMaxResourceUsage [pulsar]

Posted by "keyboardbobo (via GitHub)" <gi...@apache.org>.
keyboardbobo commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1846506274

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] HQebupt commented on pull request #16319: [Improve][broker] Use getMaxResourceUsageWithWeight instead of getMaxResourceUsage in selectBrokerForAssignment

Posted by GitBox <gi...@apache.org>.
HQebupt commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1172825233

   > 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`?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] github-actions[bot] commented on pull request #16319: [Improve][broker] When calculating resource usage, use getMaxResourceUsageWithWeight instead of getMaxResourceUsage

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1204676987

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] lordcheng10 commented on pull request #16319: [Improve][broker] Use getMaxResourceUsageWithWeight instead of getMaxResourceUsage in selectBrokerForAssignment

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1172837940

   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 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] Jason918 commented on pull request #16319: [Improve][broker] When calculating resource usage, use getMaxResourceUsageWithWeight instead of getMaxResourceUsage

Posted by GitBox <gi...@apache.org>.
Jason918 commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1172852474

   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`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] heesung-sn commented on pull request #16319: [Improve][broker] Use getMaxResourceUsageWithWeight instead of getMaxResourceUsage in selectBrokerForAssignment

Posted by GitBox <gi...@apache.org>.
heesung-sn commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1172688454

   > 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](https://github.com/apache/pulsar/pull/16319/files#diff-133d58e07f2d545ed1b11905c9ada47a5c4b0a913bf8c96ab122fdaed7e50796)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [pulsar] lordcheng10 commented on pull request #16319: [Improve][broker] Use getMaxResourceUsageWithWeight instead of getMaxResourceUsage in selectBrokerForAssignment

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #16319:
URL: https://github.com/apache/pulsar/pull/16319#issuecomment-1171960020

   @eolivelli @hanbo1990 @Shoothzj PTAL,thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org