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


##########
solr/core/src/java/org/apache/solr/util/circuitbreaker/CPUCircuitBreaker.java:
##########
@@ -33,39 +35,47 @@
  * We depend on OperatingSystemMXBean which does not allow a configurable 
interval of collection of
  * data.
  */
-public class CPUCircuitBreaker extends CircuitBreaker {
+public class CPUCircuitBreaker extends CircuitBreaker implements SolrCoreAware 
{
   private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  private boolean enabled = true;
+  private Boolean enabled = null;
   private double cpuUsageThreshold;
-  private final SolrCore core;
+  private CoreContainer cc;
 
   private static final ThreadLocal<Double> seenCPUUsage = 
ThreadLocal.withInitial(() -> 0.0);
 
   private static final ThreadLocal<Double> allowedCPUUsage = 
ThreadLocal.withInitial(() -> 0.0);
 
-  public CPUCircuitBreaker(SolrCore core) {
+  public CPUCircuitBreaker() {
     super();
-    this.core = core;
+  }
+
+  public CPUCircuitBreaker(CoreContainer coreContainer) {
+    super();
+    this.cc = coreContainer;
   }
 
   @Override
   public void init(NamedList<?> args) {
     super.init(args);
-    double localSeenCPUUsage = calculateLiveCPUUsage();
-
-    if (localSeenCPUUsage < 0) {
-      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 == null) {
+      double localSeenCPUUsage = calculateLiveCPUUsage();
+
+      if (localSeenCPUUsage < 0) {
+        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;
+      } else {
+        enabled = true;
+      }
+    }

Review Comment:
   When this CB is initialized by SolrCore it gets its CoreContainer from 
`inform()`, which happens after `init()`. Thus  had to move the early check of 
enabled true/false to here.
   
   When initialized from static / global context, we still use the constructor 
with CC, since we don't have a SolrCore there to inject.
   
   



##########
solr/core/src/java/org/apache/solr/core/SolrCore.java:
##########
@@ -3178,6 +3179,9 @@ public <T> T initPlugins(
             type.getSimpleName() + "." + info.name, (SolrMetricProducer) o);
       }
       if (o instanceof CircuitBreaker) {
+        if (o instanceof SolrCoreAware) {
+          ((SolrCoreAware) o).inform(this);

Review Comment:
   As this init mechanism requires a default no-arg constructor, had to make it 
SolrCoreAware to inject CC.



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