Fix internal bug when amount of context exceeds the threshold (#152)

- When amount of context exceeds the threshold, the `NullContext` will be set to current thread local. Thus, when checking context in `CtSph#entry`, once `NullContext` detected, the entry will not do rule checking on the slot chain. - Cache the default context during initialization. Then when amount of context exceeds the threshold, entries under default context can do rule checking under default context. - Enhance the error message Signed-off-by: Eric Zhao <[email protected]>
alibaba · Sep 19, 2018 · be43a31 · be43a31
1 parent 91f2797
commit be43a31
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 9 deletions.
diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/Constants.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/Constants.java
@@ -31,7 +31,7 @@ public class Constants {
     public final static int MAX_CONTEXT_NAME_SIZE = 2000;
     public final static int MAX_SLOT_CHAIN_SIZE = 6000;
     public final static String ROOT_ID = "machine-root";
-    public final static String CONTEXT_DEFAULT_NAME = "default_context_name";
+    public final static String CONTEXT_DEFAULT_NAME = "sentinel_default_context";
 
     public final static DefaultNode ROOT = new EntranceNode(new StringResourceWrapper(ROOT_ID, EntryType.IN),
         Env.nodeBuilder.buildClusterNode());

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
@@ -17,6 +17,7 @@
 
 import com.alibaba.csp.sentinel.context.Context;
 import com.alibaba.csp.sentinel.context.ContextUtil;
+import com.alibaba.csp.sentinel.context.NullContext;
 import com.alibaba.csp.sentinel.node.Node;
 import com.alibaba.csp.sentinel.slotchain.ProcessorSlot;
 import com.alibaba.csp.sentinel.slotchain.ResourceWrapper;
@@ -44,6 +45,10 @@ class CtEntry extends Entry {
     }
 
     private void setUpEntryFor(Context context) {
+        // The entry should not be associated to NullContext.
+        if (context instanceof NullContext) {
+            return;
+        }
         this.parent = context.getCurEntry();
         if (parent != null) {
             ((CtEntry)parent).child = this;
@@ -58,14 +63,21 @@ public void exit(int count, Object... args) throws ErrorEntryFreeException {
 
     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();
                 // Clean previous call stack.
                 CtEntry e = (CtEntry)context.getCurEntry();
                 while (e != null) {
                     e.exit(count, args);
                     e = (CtEntry)e.parent;
                 }
-                throw new ErrorEntryFreeException("The order of entry free is can't be paired with the order of entry");
+                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());
+                throw new ErrorEntryFreeException(errorMessage);
             } else {
                 if (chain != null) {
                     chain.exit(context, resourceWrapper, count, args);
@@ -76,7 +88,8 @@ protected void exitForContext(Context context, int count, Object... args) throws
                     ((CtEntry)parent).child = null;
                 }
                 if (parent == null) {
-                    // Auto-created entry indicates immediate exit.
+                    // Default context (auto entered) will be exited automatically.
+                    // Note: NullContext won't be exited automatically.
                     ContextUtil.exit();
                 }
                 // Clean the reference of context in current entry to avoid duplicate exit.

diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/CtSph.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/CtSph.java
@@ -56,7 +56,8 @@ public class CtSph implements Sph {
     private AsyncEntry asyncEntryInternal(ResourceWrapper resourceWrapper, int count, Object... args) throws BlockException {
         Context context = ContextUtil.getContext();
         if (context instanceof NullContext) {
-            // Init the entry only. No rule checking will occur.
+            // The {@link NullContext} indicates that the amount of context has exceeded the threshold,
+            // so here init the entry only. No rule checking will be done.
             return new AsyncEntry(resourceWrapper, null, context);
         }
         if (context == null) {
@@ -112,7 +113,8 @@ private AsyncEntry asyncEntryInternal(ResourceWrapper resourceWrapper, int count
     public Entry entry(ResourceWrapper resourceWrapper, int count, Object... args) throws BlockException {
         Context context = ContextUtil.getContext();
         if (context instanceof NullContext) {
-            // Init the entry only. No rule checking will occur.
+            // The {@link NullContext} indicates that the amount of context has exceeded the threshold,
+            // so here init the entry only. No rule checking will be done.
             return new CtEntry(resourceWrapper, null, context);
         }
 
@@ -142,6 +144,7 @@ public Entry entry(ResourceWrapper resourceWrapper, int count, Object... args) t
             e.exit(count, args);
             throw e1;
         } catch (Throwable e1) {
+            // This should not happen, unless there are errors existing in Sentinel internal.
             RecordLog.info("Sentinel unexpected exception", e1);
         }
         return e;

diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/context/Context.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/context/Context.java
@@ -187,4 +187,15 @@ public Node getLastNode() {
     public Node getOriginNode() {
         return curEntry == null ? null : curEntry.getOriginNode();
     }
+
+    @Override
+    public String toString() {
+        return "Context{" +
+            "name='" + name + '\'' +
+            ", entranceNode=" + entranceNode +
+            ", curEntry=" + curEntry +
+            ", origin='" + origin + '\'' +
+            ", async=" + async +
+            '}';
+    }
 }
diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/context/ContextUtil.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/context/ContextUtil.java
@@ -49,13 +49,35 @@ public class ContextUtil {
     private static ThreadLocal<Context> contextHolder = new ThreadLocal<Context>();
 
     /**
-     * Holds all {@link EntranceNode}
+     * Holds all {@link EntranceNode}.
      */
     private static volatile Map<String, DefaultNode> contextNameNodeMap = new HashMap<String, DefaultNode>();
 
     private static final ReentrantLock LOCK = new ReentrantLock();
     private static final Context NULL_CONTEXT = new NullContext();
 
+    static {
+        // Cache the entrance node for default context.
+        initDefaultContext();
+    }
+
+    private static void initDefaultContext() {
+        String defaultContextName = Constants.CONTEXT_DEFAULT_NAME;
+        EntranceNode node = new EntranceNode(new StringResourceWrapper(defaultContextName, EntryType.IN), null);
+        Constants.ROOT.addChild(node);
+        contextNameNodeMap.put(defaultContextName, node);
+    }
+
+    /**
+     * Not thread-safe, only for test.
+     */
+    static void resetContextMap() {
+        if (contextNameNodeMap != null) {
+            contextNameNodeMap.clear();
+            initDefaultContext();
+        }
+    }
+
     /**
      * <p>
      * Enter the invocation context. The context is ThreadLocal, meaning that
@@ -99,13 +121,15 @@ protected static Context trueEnter(String name, String origin) {
             DefaultNode node = localCacheNameMap.get(name);
             if (node == null) {
                 if (localCacheNameMap.size() > Constants.MAX_CONTEXT_NAME_SIZE) {
+                    contextHolder.set(NULL_CONTEXT);
                     return NULL_CONTEXT;
                 } else {
                     try {
                         LOCK.lock();
                         node = contextNameNodeMap.get(name);
                         if (node == null) {
                             if (contextNameNodeMap.size() > Constants.MAX_CONTEXT_NAME_SIZE) {
+                                contextHolder.set(NULL_CONTEXT);
                                 return NULL_CONTEXT;
                             } else {
                                 node = new EntranceNode(new StringResourceWrapper(name, EntryType.IN), null);

diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/context/NullContext.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/context/NullContext.java
@@ -27,7 +27,6 @@
 public class NullContext extends Context {
 
     public NullContext() {
-        super(null, null);
+        super(null, "null_context_internal");
     }
-
 }
diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/context/ContextTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/context/ContextTest.java
@@ -15,7 +15,11 @@
  */
 package com.alibaba.csp.sentinel.context;
 
+import com.alibaba.csp.sentinel.Constants;
+import com.alibaba.csp.sentinel.TestUtil;
+
 import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 
 import static org.junit.Assert.*;
@@ -28,9 +32,56 @@
  */
 public class ContextTest {
 
+    @Before
+    public void setUp() {
+        resetContextMap();
+    }
+
     @After
     public void cleanUp() {
-        ContextUtil.exit();
+        TestUtil.cleanUpContext();
+    }
+
+    @Test
+    public void testEnterCustomContextWhenExceedsThreshold() {
+        fillContext();
+        try {
+            String contextName = "abc";
+            ContextUtil.enter(contextName, "bcd");
+            Context curContext = ContextUtil.getContext();
+            assertNotEquals(contextName, curContext.getName());
+            assertTrue(curContext instanceof NullContext);
+            assertEquals("", curContext.getOrigin());
+        } finally {
+            ContextUtil.exit();
+            resetContextMap();
+        }
+    }
+
+    @Test
+    public void testDefaultContextWhenExceedsThreshold() {
+        fillContext();
+        try {
+            ContextUtil.trueEnter(Constants.CONTEXT_DEFAULT_NAME, "");
+            Context curContext = ContextUtil.getContext();
+            assertEquals(Constants.CONTEXT_DEFAULT_NAME, curContext.getName());
+            assertNotNull(curContext.getEntranceNode());
+        } finally {
+            ContextUtil.exit();
+            resetContextMap();
+        }
+    }
+
+    private void fillContext() {
+        for (int i = 0; i < Constants.MAX_CONTEXT_NAME_SIZE; i++) {
+            ContextUtil.enter("test-context-" + i);
+            ContextUtil.exit();
+        }
+    }
+
+    private void resetContextMap() {
+        ContextUtil.resetContextMap();
+        Constants.ROOT.removeChildList();
     }
 
     @Test