janhoy commented on code in PR #96:
URL: https://github.com/apache/solr/pull/96#discussion_r1315215619


##########
solr/core/src/java/org/apache/solr/util/circuitbreaker/CPUCircuitBreaker.java:
##########
@@ -17,54 +17,49 @@
 
 package org.apache.solr.util.circuitbreaker;
 
+import com.codahale.metrics.Gauge;
+import com.codahale.metrics.Metric;
 import java.lang.invoke.MethodHandles;
-import java.lang.management.ManagementFactory;
-import java.lang.management.OperatingSystemMXBean;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.metrics.SolrMetricManager;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
  * Tracks current CPU usage and triggers if the specified threshold is 
breached.
  *
- * <p>This circuit breaker gets the average CPU load over the last minute and 
uses that data to take
- * a decision. We depend on OperatingSystemMXBean which does not allow a 
configurable interval of
- * collection of data. //TODO: Use Codahale Meter to calculate the value 
locally.
- *
- * <p>The configuration to define which mode to use and the trigger threshold 
are defined in
- * solrconfig.xml
+ * <p>This circuit breaker gets the recent average CPU usage and uses that 
data to take a decision.
+ * We depend on OperatingSystemMXBean which does not allow a configurable 
interval of collection of
+ * data.
  */
 public class CPUCircuitBreaker extends CircuitBreaker {
   private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
-  private static final OperatingSystemMXBean operatingSystemMXBean =
-      ManagementFactory.getOperatingSystemMXBean();
 
+  private static boolean warningLogged = false;

Review Comment:
   Here's a patch that I believe does the trick. It will log an ERROR duing 
initialization and disable the plugin. On every call it will log DEBUG message 
about it being disabled:
   ```patch
   diff --git 
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/CPUCircuitBreaker.java 
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/CPUCircuitBreaker.java
   --- 
a/solr/core/src/java/org/apache/solr/util/circuitbreaker/CPUCircuitBreaker.java 
 (revision d2951a00f6427835fce82304e481a0536cc8f11c)
   +++ 
b/solr/core/src/java/org/apache/solr/util/circuitbreaker/CPUCircuitBreaker.java 
 (date 1693865423560)
   @@ -20,6 +20,7 @@
    import com.codahale.metrics.Gauge;
    import com.codahale.metrics.Metric;
    import java.lang.invoke.MethodHandles;
   +import org.apache.solr.common.util.NamedList;
    import org.apache.solr.core.SolrCore;
    import org.apache.solr.metrics.SolrMetricManager;
    import org.slf4j.Logger;
   @@ -35,7 +36,7 @@
    public class CPUCircuitBreaker extends CircuitBreaker {
      private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
    
   -  private static boolean warningLogged = false;
   +  private static boolean enabled = true;
      private double cpuUsageThreshold;
      private final SolrCore core;
    
   @@ -51,19 +52,31 @@
      }
    
      @Override
   -  public boolean isTripped() {
   -    double localAllowedCPUUsage = getCpuUsageThreshold();
   +  public void init(NamedList<?> args) {
   +    super.init(args);
        double localSeenCPUUsage = calculateLiveCPUUsage();
    
        if (localSeenCPUUsage < 0) {
   -      if (!warningLogged && log.isWarnEnabled()) {
   -        String msg = "Unable to get CPU usage";
   -        log.warn(msg);
   -        warningLogged = true;
   +      String msg =
   +          "Initialization failure for CPU circuit breaker. Unable to get 
'systemCpuLoad', not supported by the JVM?";
   +      if (log.isErrorEnabled()) {
   +        log.error(msg);
          }
   +      enabled = false;
   +    }
   +  }
    
   +  @Override
   +  public boolean isTripped() {
   +    if (!enabled) {
   +      if (log.isDebugEnabled()) {
   +        log.debug(
   +            "CPU circuit breaker is disabled due to initialization failure, 
will never trip.");
   +      }
          return false;
        }
   +    double localAllowedCPUUsage = getCpuUsageThreshold();
   +    double localSeenCPUUsage = calculateLiveCPUUsage();
    
        allowedCPUUsage.set(localAllowedCPUUsage);
   
   ```



-- 
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: issues-unsubscr...@solr.apache.org

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


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

Reply via email to