Fix the problem that requests will never be blocked when errRatioThre…

…shold = 100%(alibaba#1856)
xierunzi · Jul 17, 2022 · 99a2e64 · 99a2e64
1 parent 9f08a6a
commit 99a2e64
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 5 deletions.
diff --git a/...a/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/AbstractCircuitBreaker.java b/...a/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/AbstractCircuitBreaker.java
@@ -31,6 +31,7 @@
  * @since 1.8.0
  */
 public abstract class AbstractCircuitBreaker implements CircuitBreaker {
+    protected static final double MAX_RATIO = 1.0d;
 
     protected final DegradeRule rule;
     protected final int recoveryTimeoutMs;
@@ -122,7 +123,7 @@ public void accept(Context context, Entry entry) {
         }
         return false;
     }
-    
+
     private void notifyObservers(CircuitBreaker.State prevState, CircuitBreaker.State newState, Double snapshotValue) {
         for (CircuitBreakerStateChangeObserver observer : observerRegistry.getStateChangeObservers()) {
             observer.onStateChange(prevState, newState, rule, snapshotValue);

diff --git a/.../com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ExceptionCircuitBreaker.java b/.../com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ExceptionCircuitBreaker.java
@@ -106,6 +106,11 @@ private void handleStateChangeWhenThresholdExceeded(Throwable error) {
         if (strategy == DEGRADE_GRADE_EXCEPTION_RATIO) {
             // Use errorRatio
             curCount = errCount * 1.0d / totalCount;
+            // special case when ratio equals 1.0
+            if (Double.compare(curCount, threshold) == 0 && Double.compare(threshold, MAX_RATIO) == 0) {
+                transformToOpen(curCount);
+                return;
+            }
         }
         if (curCount > threshold) {
             transformToOpen(curCount);

diff --git a/...m/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ResponseTimeCircuitBreaker.java b/...m/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ResponseTimeCircuitBreaker.java
@@ -32,9 +32,6 @@
  * @since 1.8.0
  */
 public class ResponseTimeCircuitBreaker extends AbstractCircuitBreaker {
-
-    private static final double SLOW_REQUEST_RATIO_MAX_VALUE = 1.0d;
-
     private final long maxAllowedRt;
     private final double maxSlowRequestRatio;
     private final int minRequestAmount;
@@ -112,7 +109,7 @@ private void handleStateChangeWhenThresholdExceeded(long rt) {
             transformToOpen(currentRatio);
         }
         if (Double.compare(currentRatio, maxSlowRequestRatio) == 0 &&
-                Double.compare(maxSlowRequestRatio, SLOW_REQUEST_RATIO_MAX_VALUE) == 0) {
+                Double.compare(maxSlowRequestRatio, MAX_RATIO) == 0) {
             transformToOpen(currentRatio);
         }
     }

diff --git a/.../alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ExceptionCircuitBreakerTest.java b/.../alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ExceptionCircuitBreakerTest.java
@@ -20,6 +20,7 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 
 import org.junit.After;
 import org.junit.Before;
@@ -82,4 +83,29 @@ public void testRecordErrorOrSuccess() throws BlockException {
         assertTrue(entryWithErrorIfPresent(resource, new IllegalArgumentException()));
         assertTrue(entryAndSleepFor(resource, 100));
     }
+
+    @Test
+    public void testMaxErrorRatioThreshold() {
+        String resource = "testMaxErrorRatioThreshold";
+        DegradeRule rule = new DegradeRule("resource")
+                .setCount(1)
+                .setGrade(RuleConstant.DEGRADE_GRADE_EXCEPTION_RATIO)
+                .setMinRequestAmount(3)
+                .setStatIntervalMs(5000)
+                .setTimeWindow(5);
+        rule.setResource(resource);
+        DegradeRuleManager.loadRules(Collections.singletonList(rule));
+
+        assertTrue(entryWithErrorIfPresent(resource, new RuntimeException()));
+        assertTrue(entryWithErrorIfPresent(resource, new RuntimeException()));
+        assertTrue(entryWithErrorIfPresent(resource, new RuntimeException()));
+
+        // should be blocked, cause 3/3 requests' rt is bigger than max rt.
+        assertFalse(entryWithErrorIfPresent(resource, new RuntimeException()));
+        assertFalse(entryWithErrorIfPresent(resource, new RuntimeException()));
+
+        sleep(5000);
+
+        assertTrue(entryWithErrorIfPresent(resource, new RuntimeException()));
+    }
 }