From dae4621e6e41576fb6133b01426a721ae3a9260e Mon Sep 17 00:00:00 2001 From: Eric Zhao Date: Tue, 18 Aug 2020 16:42:58 +0800 Subject: [PATCH] Refactor exit handler mechanism of Entry - Rename: whenComplete -> whenTerminate - Execute the exit handler directly after the onExit hook of all slots Signed-off-by: Eric Zhao --- .../com/alibaba/csp/sentinel/CtEntry.java | 58 +++++++++++-------- .../java/com/alibaba/csp/sentinel/Entry.java | 9 +-- .../AbstractCircuitBreaker.java | 2 +- .../com/alibaba/csp/sentinel/EntryTest.java | 2 +- 4 files changed, 40 insertions(+), 31 deletions(-) diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/CtEntry.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/CtEntry.java index 9b491743..6c62abe8 100644 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/CtEntry.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/CtEntry.java @@ -15,7 +15,6 @@ */ package com.alibaba.csp.sentinel; -import java.util.Iterator; import java.util.LinkedList; import com.alibaba.csp.sentinel.context.Context; @@ -41,7 +40,6 @@ class CtEntry extends Entry { protected ProcessorSlot chain; protected Context context; protected LinkedList> exitHandlers; - CtEntry(ResourceWrapper resourceWrapper, ProcessorSlot chain, Context context) { super(resourceWrapper); @@ -58,7 +56,7 @@ class CtEntry extends Entry { } this.parent = context.getCurEntry(); if (parent != null) { - ((CtEntry)parent).child = this; + ((CtEntry) parent).child = this; } context.setCurEntry(this); } @@ -68,31 +66,55 @@ class CtEntry extends Entry { trueExit(count, args); } + /** + * Note: the exit handlers will be called AFTER onExit of slot chain. + */ + private void callExitHandlersAndCleanUp(Context ctx) { + if (exitHandlers != null && !exitHandlers.isEmpty()) { + for (BiConsumer handler : this.exitHandlers) { + try { + handler.accept(ctx, this); + } catch (Exception e) { + RecordLog.warn("Error occurred when invoking entry exit handler, current entry: " + + resourceWrapper.getName(), e); + } + } + exitHandlers = null; + } + } + protected void exitForContext(Context context, int count, Object... args) throws ErrorEntryFreeException { if (context != null) { // Null context should exit without clean-up. if (context instanceof NullContext) { return; } + if (context.getCurEntry() != this) { - String curEntryNameInContext = context.getCurEntry() == null ? null : context.getCurEntry().getResourceWrapper().getName(); + String curEntryNameInContext = context.getCurEntry() == null ? null + : context.getCurEntry().getResourceWrapper().getName(); // Clean previous call stack. - CtEntry e = (CtEntry)context.getCurEntry(); + CtEntry e = (CtEntry) context.getCurEntry(); while (e != null) { e.exit(count, args); - e = (CtEntry)e.parent; + e = (CtEntry) e.parent; } String errorMessage = String.format("The order of entry exit can't be paired with the order of entry" - + ", current entry in context: <%s>, but expected: <%s>", curEntryNameInContext, resourceWrapper.getName()); + + ", current entry in context: <%s>, but expected: <%s>", curEntryNameInContext, + resourceWrapper.getName()); throw new ErrorEntryFreeException(errorMessage); } else { + // Go through the onExit hook of all slots. if (chain != null) { chain.exit(context, resourceWrapper, count, args); } + // Go through the existing terminate handlers (associated to this invocation). + callExitHandlersAndCleanUp(context); + // Restore the call stack. context.setCurEntry(parent); if (parent != null) { - ((CtEntry)parent).child = null; + ((CtEntry) parent).child = null; } if (parent == null) { // Default context (auto entered) will be exited automatically. @@ -109,32 +131,18 @@ class CtEntry extends Entry { protected void clearEntryContext() { this.context = null; } - + @Override - public void whenComplete(BiConsumer consumer) { + public void whenTerminate(BiConsumer handler) { if (this.exitHandlers == null) { this.exitHandlers = new LinkedList<>(); } - this.exitHandlers.add(consumer); + this.exitHandlers.add(handler); } @Override protected Entry trueExit(int count, Object... args) throws ErrorEntryFreeException { exitForContext(context, count, args); - - if (this.exitHandlers != null) { - Iterator> it = this.exitHandlers.iterator(); - BiConsumer cur; - while (it.hasNext()) { - cur = it.next(); - try { - cur.accept(this.context, this); - } catch (Exception e) { - RecordLog.warn("Error invoking exit handler", e); - } - } - this.exitHandlers = null; - } return parent; } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/Entry.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/Entry.java index fbe19a14..c3114821 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/Entry.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/Entry.java @@ -180,12 +180,13 @@ public abstract class Entry implements AutoCloseable { } /** - * Like `CompletableFuture` since JDK8 it guarantees specified consumer - * is invoked when this entry exited. + * Like {@code CompletableFuture} since JDK 8, it guarantees specified handler + * is invoked when this entry terminated (exited), no matter it's blocked or permitted. * Use it when you did some STATEFUL operations on entries. * - * @param consumer + * @param handler handler function on the invocation terminates + * @since 1.8.0 */ - public abstract void whenComplete(BiConsumer consumer); + public abstract void whenTerminate(BiConsumer handler); } 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 094f3eef..86be556f 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 @@ -105,7 +105,7 @@ public abstract class AbstractCircuitBreaker implements CircuitBreaker { if (currentState.compareAndSet(State.OPEN, State.HALF_OPEN)) { notifyObservers(State.OPEN, State.HALF_OPEN, null); Entry entry = context.getCurEntry(); - entry.whenComplete(new BiConsumer() { + entry.whenTerminate(new BiConsumer() { @Override public void accept(Context context, Entry entry) { // Note: This works as a temporary workaround for https://github.com/alibaba/Sentinel/issues/1638 diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/EntryTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/EntryTest.java index 237fcbc1..78b56501 100644 --- a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/EntryTest.java +++ b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/EntryTest.java @@ -68,7 +68,7 @@ public class EntryTest { } @Override - public void whenComplete(BiConsumer consumer) { + public void whenTerminate(BiConsumer consumer) { // do nothing } }