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