From 55e038cc33cfe47b659ac78e84a3fa69197393be Mon Sep 17 00:00:00 2001
From: Eric Zhao <sczyh16@gmail.com>
Date: Tue, 18 Aug 2020 15:26:52 +0800
Subject: [PATCH] Polish CircuitBreaker interface and update comments

- Only carry context in tryPass/onComplete method (this might be generic in upcoming versions)

Signed-off-by: Eric Zhao <sczyh16@gmail.com>
---
 .../csp/sentinel/slots/block/degrade/DegradeSlot.java |  4 ++--
 .../circuitbreaker/AbstractCircuitBreaker.java        | 10 +++++-----
 .../block/degrade/circuitbreaker/CircuitBreaker.java  | 11 +++++------
 .../circuitbreaker/ExceptionCircuitBreaker.java       |  2 +-
 .../circuitbreaker/ResponseTimeCircuitBreaker.java    |  2 +-
 5 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeSlot.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeSlot.java
index 32c05018..c66d056d 100755
--- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeSlot.java
+++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeSlot.java
@@ -50,7 +50,7 @@ public class DegradeSlot extends AbstractLinkedProcessorSlot<DefaultNode> {
             return;
         }
         for (CircuitBreaker cb : circuitBreakers) {
-            if (!cb.tryPass(context, r)) {
+            if (!cb.tryPass(context)) {
                 throw new DegradeException(cb.getRule().getLimitApp(), cb.getRule());
             }
         }
@@ -72,7 +72,7 @@ public class DegradeSlot extends AbstractLinkedProcessorSlot<DefaultNode> {
         if (curEntry.getBlockError() == null) {
             // passed request
             for (CircuitBreaker circuitBreaker : circuitBreakers) {
-                circuitBreaker.onRequestComplete(context, r);
+                circuitBreaker.onRequestComplete(context);
             }
         }
 
diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/AbstractCircuitBreaker.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/AbstractCircuitBreaker.java
index c516b66b..094f3eef 100644
--- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/AbstractCircuitBreaker.java
+++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/AbstractCircuitBreaker.java
@@ -65,13 +65,13 @@ public abstract class AbstractCircuitBreaker implements CircuitBreaker {
     }
 
     @Override
-    public boolean tryPass(Context context, ResourceWrapper r) {
+    public boolean tryPass(Context context) {
         // Template implementation.
         if (currentState.get() == State.CLOSED) {
             return true;
         }
         if (currentState.get() == State.OPEN) {
-            // For half-open state we allow a request for trial.
+            // For half-open state we allow a request for probing.
             return retryTimeoutArrived() && fromOpenToHalfOpen(context);
         }
         return false;
@@ -106,16 +106,16 @@ public abstract class AbstractCircuitBreaker implements CircuitBreaker {
             notifyObservers(State.OPEN, State.HALF_OPEN, null);
             Entry entry = context.getCurEntry();
             entry.whenComplete(new BiConsumer<Context, Entry>() {
-                
                 @Override
                 public void accept(Context context, Entry entry) {
+                    // Note: This works as a temporary workaround for https://github.com/alibaba/Sentinel/issues/1638
+                    // Without the hook, the circuit breaker won't recover from half-open state in some circumstances
+                    // when the request is actually blocked by upcoming rules (not only degrade rules).
                     if (entry.getBlockError() != null) {
                         // Fallback to OPEN due to detecting request is blocked
                         currentState.compareAndSet(State.HALF_OPEN, State.OPEN);
                         notifyObservers(State.HALF_OPEN, State.OPEN, 1.0d);
-                        return;
                     }
-                    
                 }
             });
             return true;
diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/CircuitBreaker.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/CircuitBreaker.java
index cc4b349b..0ecc26c6 100644
--- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/CircuitBreaker.java
+++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/CircuitBreaker.java
@@ -36,11 +36,10 @@ public interface CircuitBreaker {
     /**
      * Acquires permission of an invocation only if it is available at the time of invoking.
      *
-     * @param context
-     * @param r
+     * @param context context of current invocation
      * @return {@code true} if permission was acquired and {@code false} otherwise
      */
-    boolean tryPass(Context context, ResourceWrapper r);
+    boolean tryPass(Context context);
 
     /**
      * Get current state of the circuit breaker.
@@ -50,12 +49,12 @@ public interface CircuitBreaker {
     State currentState();
 
     /**
-     * Called when a `passed` invocation finished.
+     * <p>Record a completed request with the context and handle state transformation of the circuit breaker.</p>
+     * <p>Called when a <strong>passed</strong> invocation finished.</p>
      *
      * @param context context of current invocation
-     * @param wrapper current resource
      */
-    void onRequestComplete(Context context, ResourceWrapper wrapper);
+    void onRequestComplete(Context context);
 
     /**
      * Circuit breaker state.
diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ExceptionCircuitBreaker.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ExceptionCircuitBreaker.java
index fa286f72..be8c1b20 100644
--- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ExceptionCircuitBreaker.java
+++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ExceptionCircuitBreaker.java
@@ -63,7 +63,7 @@ public class ExceptionCircuitBreaker extends AbstractCircuitBreaker {
     }
 
     @Override
-    public void onRequestComplete(Context context, ResourceWrapper r) {
+    public void onRequestComplete(Context context) {
         Entry entry = context.getCurEntry();
         if (entry == null) {
             return;
diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ResponseTimeCircuitBreaker.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ResponseTimeCircuitBreaker.java
index 7ff9e182..51f81316 100644
--- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ResponseTimeCircuitBreaker.java
+++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/circuitbreaker/ResponseTimeCircuitBreaker.java
@@ -61,7 +61,7 @@ public class ResponseTimeCircuitBreaker extends AbstractCircuitBreaker {
     }
 
     @Override
-    public void onRequestComplete(Context context, ResourceWrapper wrapper) {
+    public void onRequestComplete(Context context) {
         SlowRequestCounter counter = slidingCounter.currentWindow().value();
         Entry entry = context.getCurEntry();
         if (entry == null) {