[GitHub] [solr] wrunderwood commented on pull request #96: SOLR-15056: add circuit breaker for CPU, fix load circuit breaker

[GitBox <gi...@apache.org>]

wrunderwood commented on pull request #96:
URL: https://github.com/apache/solr/pull/96#issuecomment-853347376


   That is what this patch does, move the existing behavior to a load average circuit breaker and create a new CPU usage circuit breaker.
   
   Combining the two has a problem. The range of values for the two metrics are quite different. CPU usage is limited to 0 to 1.0. Load average is unbounded, and useful limits are much greater than 1.0, typically set at the number of CPUs in the system. On a 32-way system, a good limit would be around 32.
   
   Also, load average is not a CPU metric. It is a “system load” metric. On Linux, it includes processes waiting on IO.
   
   I don’t think we need to worry about existing users. With the load average (“CPU”) circuit breaker limited to 1.0, it is not usable.
   
   wunder
   Walter Underwood
   ***@***.***
   http://observer.wunderwood.org/  (my blog)
   
   > On Jun 2, 2021, at 12:33 PM, Christine Poerschke ***@***.***> wrote:
   > 
   > 
   > So one possibility is to change the implementation of the existing CPUCircuitBreaker class and to add a new LoadAverageCircuitBreaker class which has the same behaviour as the existing CPUCircuitBreaker class. Changing an existing class's behaviour may bring backwards compatibility concerns and there being two related circuit breaker classes may raise questions of code duplication and whether or not to factor out a common base class.
   > 
   > I wonder if another possibility could be to have a single CPUCircuitBreaker class and for a configurable attribute to determine the implementation's behaviour:
   > 
   > Existing users with a configuration like this
   > 
   > <circuitBreaker class="solr.CircuitBreakerManager" enabled="true">
   >   <str name="cpuEnabled">true</str>
   >   <str name="cpuThreshold">42</str>
   > </circuitBreaker>
   > would continue to get their existing behaviour, equivalent to
   > 
   > <circuitBreaker class="solr.CircuitBreakerManager" enabled="true">
   >   <str name="cpuEnabled">true</str>
   >   <str name="cpuThreshold">42</str>
   >   <str name="cpuMethod">OperatingSystemMXBean</str>
   > </circuitBreaker>
   > configuration and users wishing to use the new alternative implementation can do so via
   > 
   > <circuitBreaker class="solr.CircuitBreakerManager" enabled="true">
   >   <str name="cpuEnabled">true</str>
   >   <str name="cpuThreshold">7</str>
   >   <str name="cpuMethod">SolrMetricManager</str>
   > </circuitBreaker>
   > configuration.
   > 
   > —
   > You are receiving this because you authored the thread.
   > Reply to this email directly, view it on GitHub <https://github.com/apache/solr/pull/96#issuecomment-853328967>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACR4IINBRUWYVWNQNUBJ6NTTQ2BSPANCNFSM43T5FIZA>.
   > 
   
   


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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org