SOLR-15474 Make Circuit breakers pluggable (take 2) (apache#1725)

* SOLR-15474 Make Circuit breakers pluggable Co-authored-by: Christine Poerschke <[email protected]> Co-authored-by: Atri Sharma <[email protected]> Signed-off-by: Jan Høydahl <[email protected]>
cowpaths · Aug 29, 2023 · 4100049 · 4100049
1 parent 7a88bd8
commit 4100049
Show file tree

Hide file tree

Showing 18 changed files with 693 additions and 643 deletions.
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
@@ -103,6 +103,8 @@ Improvements
 
 * SOLR-16940: Users can pass Java system properties to the SolrCLI via the SOLR_TOOL_OPTS environment variable. (Houston Putman)
 
+* SOLR-15474: Make Circuit breakers individually pluggable (Atri Sharma, Christine Poerschke, janhoy)
+
 * SOLR-16927: Allow SolrClientCache clients to use Jetty HTTP2 clients (Alex Deparvu, David Smiley)
 
 Optimizations
@@ -288,7 +290,7 @@ Improvements
 
 * SOLR-16687: Add support of SolrClassLoader to SolrZkClient (Lamine Idjeraoui via Jason Gerlowski & Houston Putman)
 
-* SOLR-9378: Internal shard requests no longer include the wasteful shard.url param.  [shard] transformer now defaults to returning 
+* SOLR-9378: Internal shard requests no longer include the wasteful shard.url param.  [shard] transformer now defaults to returning
   only the shard id (based on luceneMatchVersion), but can be configured to return the legacy list of replicas. (hossman)
 
 * SOLR-16816: Update node metrics while making affinityPlacement selections. Therefore selections can be made given the expected cluster

diff --git a/solr/core/src/java/org/apache/solr/core/SolrConfig.java b/solr/core/src/java/org/apache/solr/core/SolrConfig.java
@@ -90,7 +90,7 @@
 import org.apache.solr.update.processor.UpdateRequestProcessorFactory;
 import org.apache.solr.util.DOMConfigNode;
 import org.apache.solr.util.DataConfigNode;
-import org.apache.solr.util.circuitbreaker.CircuitBreakerManager;
+import org.apache.solr.util.circuitbreaker.CircuitBreaker;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -503,7 +503,7 @@ public static final Version parseLuceneVersionString(final String matchVersion)
           new SolrPluginInfo(IndexSchemaFactory.class, "schemaFactory", REQUIRE_CLASS),
           new SolrPluginInfo(RestManager.class, "restManager"),
           new SolrPluginInfo(StatsCache.class, "statsCache", REQUIRE_CLASS),
-          new SolrPluginInfo(CircuitBreakerManager.class, "circuitBreaker"));
+          new SolrPluginInfo(CircuitBreaker.class, "circuitBreaker", REQUIRE_CLASS, MULTI_OK));
   public static final Map<String, SolrPluginInfo> classVsSolrPluginInfo;
 
   static {

diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -173,7 +173,8 @@
 import org.apache.solr.util.PropertiesOutputStream;
 import org.apache.solr.util.RefCounted;
 import org.apache.solr.util.TestInjection;
-import org.apache.solr.util.circuitbreaker.CircuitBreakerManager;
+import org.apache.solr.util.circuitbreaker.CircuitBreaker;
+import org.apache.solr.util.circuitbreaker.CircuitBreakerRegistry;
 import org.apache.solr.util.plugin.NamedListInitializedPlugin;
 import org.apache.solr.util.plugin.PluginInfoInitialized;
 import org.apache.solr.util.plugin.SolrCoreAware;
@@ -245,7 +246,7 @@ public class SolrCore implements SolrInfoBean, Closeable {
   private final ConfigSet configSet;
   // singleton listener for all packages used in schema
 
-  private final CircuitBreakerManager circuitBreakerManager;
+  private final CircuitBreakerRegistry circuitBreakerRegistry = new CircuitBreakerRegistry();
 
   private final List<Runnable> confListeners = new CopyOnWriteArrayList<>();
 
@@ -1084,10 +1085,12 @@ private SolrCore(
       this.configSetProperties = configSet.getProperties();
       // Initialize the metrics manager
       this.coreMetricManager = initCoreMetricManager(solrConfig);
-      this.circuitBreakerManager = initCircuitBreakerManager();
       solrMetricsContext = coreMetricManager.getSolrMetricsContext();
       this.coreMetricManager.loadReporters();
 
+      // init pluggable circuit breakers
+      initPlugins(null, CircuitBreaker.class);
+
       if (updateHandler == null) {
         directoryFactory = initDirectoryFactory();
         recoveryStrategyBuilder = initRecoveryStrategyBuilder();
@@ -1324,13 +1327,6 @@ private SolrCoreMetricManager initCoreMetricManager(SolrConfig config) {
     return coreMetricManager;
   }
 
-  private CircuitBreakerManager initCircuitBreakerManager() {
-    final PluginInfo info = solrConfig.getPluginInfo(CircuitBreakerManager.class.getName());
-    CircuitBreakerManager circuitBreakerManager = CircuitBreakerManager.build(info);
-
-    return circuitBreakerManager;
-  }
-
   @Override
   public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
     newSearcherCounter = parentContext.counter("new", Category.SEARCHER.toString());
@@ -1710,8 +1706,8 @@ public PluginBag<UpdateRequestProcessorFactory> getUpdateProcessors() {
     return updateProcessors;
   }
 
-  public CircuitBreakerManager getCircuitBreakerManager() {
-    return circuitBreakerManager;
+  public CircuitBreakerRegistry getCircuitBreakerRegistry() {
+    return circuitBreakerRegistry;
   }
 
   // this core current usage count
@@ -3164,11 +3160,14 @@ public <T> T initPlugins(
     T def = null;
     for (PluginInfo info : pluginInfos) {
       T o = createInitInstance(info, type, type.getSimpleName(), defClassName);
-      registry.put(info.name, o);
+      if (registry != null) registry.put(info.name, o);
       if (o instanceof SolrMetricProducer) {
         coreMetricManager.registerMetricProducer(
             type.getSimpleName() + "." + info.name, (SolrMetricProducer) o);
       }
+      if (o instanceof CircuitBreaker) {
+        circuitBreakerRegistry.register((CircuitBreaker) o);
+      }
       if (info.isDefault()) {
         def = o;
       }

diff --git a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
@@ -97,6 +97,7 @@ public class SolrResourceLoader
     "request.",
     "update.processor.",
     "util.",
+    "util.circuitbreaker.",
     "spelling.",
     "handler.component.",
     "spelling.suggest.",

diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
@@ -67,7 +67,7 @@
 import org.apache.solr.util.RTimerTree;
 import org.apache.solr.util.SolrPluginUtils;
 import org.apache.solr.util.circuitbreaker.CircuitBreaker;
-import org.apache.solr.util.circuitbreaker.CircuitBreakerManager;
+import org.apache.solr.util.circuitbreaker.CircuitBreakerRegistry;
 import org.apache.solr.util.plugin.PluginInfoInitialized;
 import org.apache.solr.util.plugin.SolrCoreAware;
 import org.slf4j.Logger;
@@ -354,23 +354,23 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
 
     final RTimerTree timer = rb.isDebug() ? req.getRequestTimer() : null;
 
-    final CircuitBreakerManager circuitBreakerManager = req.getCore().getCircuitBreakerManager();
-    if (circuitBreakerManager.isEnabled()) {
+    final CircuitBreakerRegistry circuitBreakerRegistry = req.getCore().getCircuitBreakerRegistry();
+    if (circuitBreakerRegistry.isEnabled()) {
       List<CircuitBreaker> trippedCircuitBreakers;
 
       if (timer != null) {
         RTimerTree subt = timer.sub("circuitbreaker");
         rb.setTimer(subt);
 
-        trippedCircuitBreakers = circuitBreakerManager.checkTripped();
+        trippedCircuitBreakers = circuitBreakerRegistry.checkTripped();
 
         rb.getTimer().stop();
       } else {
-        trippedCircuitBreakers = circuitBreakerManager.checkTripped();
+        trippedCircuitBreakers = circuitBreakerRegistry.checkTripped();
       }
 
       if (trippedCircuitBreakers != null) {
-        String errorMessage = CircuitBreakerManager.toErrorMessage(trippedCircuitBreakers);
+        String errorMessage = CircuitBreakerRegistry.toErrorMessage(trippedCircuitBreakers);
         rsp.add(STATUS, FAILURE);
         rsp.setException(
             new SolrException(

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
@@ -38,31 +38,24 @@ public class CPUCircuitBreaker extends CircuitBreaker {
   private static final OperatingSystemMXBean operatingSystemMXBean =
       ManagementFactory.getOperatingSystemMXBean();
 
-  private final boolean enabled;
-  private final double cpuUsageThreshold;
+  private double cpuUsageThreshold;
 
   // Assumption -- the value of these parameters will be set correctly before invoking
   // getDebugInfo()
   private static final ThreadLocal<Double> seenCPUUsage = ThreadLocal.withInitial(() -> 0.0);
 
   private static final ThreadLocal<Double> allowedCPUUsage = ThreadLocal.withInitial(() -> 0.0);
 
-  public CPUCircuitBreaker(CircuitBreakerConfig config) {
-    super(config);
+  public CPUCircuitBreaker() {
+    super();
+  }
 
-    this.enabled = config.getCpuCBEnabled();
-    this.cpuUsageThreshold = config.getCpuCBThreshold();
+  public void setThreshold(double threshold) {
+    this.cpuUsageThreshold = threshold;
   }
 
   @Override
   public boolean isTripped() {
-    if (!isEnabled()) {
-      return false;
-    }
-
-    if (!enabled) {
-      return false;
-    }
 
     double localAllowedCPUUsage = getCpuUsageThreshold();
     double localSeenCPUUsage = calculateLiveCPUUsage();

diff --git a/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java b/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreaker.java
@@ -17,32 +17,32 @@
 
 package org.apache.solr.util.circuitbreaker;
 
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.util.SolrPluginUtils;
+import org.apache.solr.util.plugin.NamedListInitializedPlugin;
+
 /**
- * Default class to define circuit breakers for Solr.
+ * Default base class to define circuit breaker plugins for Solr. <b>Still experimental, may
+ * change</b>
  *
- * <p>There are two (typical) ways to use circuit breakers: 1. Have them checked at admission
- * control by default (use CircuitBreakerManager for the same). 2. Use the circuit breaker in a
- * specific code path(s).
+ * <p>There are two (typical) ways to use circuit breakers:
  *
- * <p>TODO: This class should be grown as the scope of circuit breakers grow.
+ * <ol>
+ *   <li>Have them checked at admission control by default (use CircuitBreakerRegistry for the
+ *       same).
+ *   <li>Use the circuit breaker in a specific code path(s).
+ * </ol>
  *
- * <p>The class and its derivatives raise a standard exception when a circuit breaker is triggered.
- * We should make it into a dedicated exception (https://issues.apache.org/jira/browse/SOLR-14755)
+ * @lucene.experimental
  */
-public abstract class CircuitBreaker {
-  public static final String NAME = "circuitbreaker";
-
-  protected final CircuitBreakerConfig config;
+public abstract class CircuitBreaker implements NamedListInitializedPlugin {
 
-  public CircuitBreaker(CircuitBreakerConfig config) {
-    this.config = config;
+  @Override
+  public void init(NamedList<?> args) {
+    SolrPluginUtils.invokeSetters(this, args);
   }
 
-  // Global config for all circuit breakers. For specific circuit breaker configs, define
-  // your own config.
-  protected boolean isEnabled() {
-    return config.isEnabled();
-  }
+  public CircuitBreaker() {}
 
   /** Check if circuit breaker is tripped. */
   public abstract boolean isTripped();
@@ -52,45 +52,4 @@ protected boolean isEnabled() {
 
   /** Get error message when the circuit breaker triggers */
   public abstract String getErrorMessage();
-
-  public static class CircuitBreakerConfig {
-    private final boolean enabled;
-    private final boolean memCBEnabled;
-    private final int memCBThreshold;
-    private final boolean cpuCBEnabled;
-    private final int cpuCBThreshold;
-
-    public CircuitBreakerConfig(
-        final boolean enabled,
-        final boolean memCBEnabled,
-        final int memCBThreshold,
-        final boolean cpuCBEnabled,
-        final int cpuCBThreshold) {
-      this.enabled = enabled;
-      this.memCBEnabled = memCBEnabled;
-      this.memCBThreshold = memCBThreshold;
-      this.cpuCBEnabled = cpuCBEnabled;
-      this.cpuCBThreshold = cpuCBThreshold;
-    }
-
-    public boolean isEnabled() {
-      return enabled;
-    }
-
-    public boolean getMemCBEnabled() {
-      return memCBEnabled;
-    }
-
-    public int getMemCBThreshold() {
-      return memCBThreshold;
-    }
-
-    public boolean getCpuCBEnabled() {
-      return cpuCBEnabled;
-    }
-
-    public int getCpuCBThreshold() {
-      return cpuCBThreshold;
-    }
-  }
 }