Adding debug mode to circuit breakers to be able to evaluate behavior (…

…#153) * Adding debug mode to circuit breakers to be able to evaluate behavior * Fixing check issues
cowpaths · Oct 4, 2023 · b0bfe4a · b0bfe4a
1 parent 9738587
commit b0bfe4a
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 7 deletions.
diff --git a/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java b/solr/core/src/java/org/apache/solr/util/circuitbreaker/CircuitBreakerManager.java
@@ -18,11 +18,14 @@
 package org.apache.solr.util.circuitbreaker;
 
 import com.google.common.annotations.VisibleForTesting;
+import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
 import java.util.List;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.core.PluginInfo;
 import org.apache.solr.util.plugin.PluginInfoInitialized;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Manages all registered circuit breaker instances. Responsible for a holistic view of whether a
@@ -39,13 +42,18 @@
  * term solution. There will be a follow up with a SIP for a schema API design.
  */
 public class CircuitBreakerManager implements PluginInfoInitialized {
+
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
   // Class private to potentially allow "family" of circuit breakers to be enabled or disabled
   private final boolean enableCircuitBreakerManager;
 
+  private final boolean debugMode;
+
   private final List<CircuitBreaker> circuitBreakerList = new ArrayList<>();
 
-  public CircuitBreakerManager(final boolean enableCircuitBreakerManager) {
+  public CircuitBreakerManager(final boolean enableCircuitBreakerManager, final boolean debugMode) {
     this.enableCircuitBreakerManager = enableCircuitBreakerManager;
+    this.debugMode = debugMode;
   }
 
   @Override
@@ -78,11 +86,20 @@ public List<CircuitBreaker> checkTripped() {
     if (enableCircuitBreakerManager) {
       for (CircuitBreaker circuitBreaker : circuitBreakerList) {
         if (circuitBreaker.isEnabled() && circuitBreaker.isTripped()) {
-          if (triggeredCircuitBreakers == null) {
-            triggeredCircuitBreakers = new ArrayList<>();
+          if (debugMode) {
+            if (log.isInfoEnabled()) {
+              log.info(
+                  "Circuit tripped for {} with debug info {}",
+                  circuitBreaker.getClass().getName(),
+                  circuitBreaker.getDebugInfo());
+            }
+          } else {
+            if (triggeredCircuitBreakers == null) {
+              triggeredCircuitBreakers = new ArrayList<>();
+            }
+
+            triggeredCircuitBreakers.add(circuitBreaker);
           }
-
-          triggeredCircuitBreakers.add(circuitBreaker);
         }
       }
     }
@@ -100,7 +117,16 @@ public boolean checkAnyTripped() {
     if (enableCircuitBreakerManager) {
       for (CircuitBreaker circuitBreaker : circuitBreakerList) {
         if (circuitBreaker.isEnabled() && circuitBreaker.isTripped()) {
-          return true;
+          if (debugMode) {
+            if (log.isInfoEnabled()) {
+              log.info(
+                  "Circuit tripped for {} with debug info {}",
+                  circuitBreaker.getClass().getName(),
+                  circuitBreaker.getDebugInfo());
+            }
+          } else {
+            return true;
+          }
         }
       }
     }
@@ -136,7 +162,11 @@ public static CircuitBreakerManager build(PluginInfo pluginInfo) {
         pluginInfo == null
             ? false
             : Boolean.parseBoolean(pluginInfo.attributes.getOrDefault("enabled", "false"));
-    CircuitBreakerManager circuitBreakerManager = new CircuitBreakerManager(enabled);
+    boolean debugMode =
+        pluginInfo == null
+            ? false
+            : Boolean.parseBoolean(pluginInfo.attributes.getOrDefault("debugMode", "false"));
+    CircuitBreakerManager circuitBreakerManager = new CircuitBreakerManager(enabled, debugMode);
 
     circuitBreakerManager.init(pluginInfo);
 

diff --git a/solr/core/src/test/org/apache/solr/util/TestCircuitBreaker.java b/solr/core/src/test/org/apache/solr/util/TestCircuitBreaker.java
@@ -215,6 +215,21 @@ public void testFakeCPUCircuitBreaker() {
     }
   }
 
+  public void testCircuitBreakerDebug() {
+    CircuitBreakerManager circuitBreakerManager = new CircuitBreakerManager(true, true);
+    PluginInfo pluginInfo =
+        h.getCore().getSolrConfig().getPluginInfo(CircuitBreakerManager.class.getName());
+
+    CircuitBreaker.CircuitBreakerConfig circuitBreakerConfig =
+        CircuitBreakerManager.buildCBConfig(pluginInfo);
+    CircuitBreaker circuitBreaker = new FakeCPUCircuitBreaker(circuitBreakerConfig);
+    circuitBreakerManager.register(circuitBreaker);
+
+    assertFalse(circuitBreakerManager.checkAnyTripped());
+
+    assertNull(circuitBreakerManager.checkTripped());
+  }
+
   private void removeAllExistingCircuitBreakers() {
     List<CircuitBreaker> registeredCircuitBreakers =
         h.getCore().getCircuitBreakerManager().getRegisteredCircuitBreakers();