From 7bf2a809e9ea60c38a22662a9617dcc4721f67c2 Mon Sep 17 00:00:00 2001 From: Eric Zhao Date: Wed, 21 Nov 2018 17:37:43 +0800 Subject: [PATCH] Code and javadoc improvement Signed-off-by: Eric Zhao --- .../csp/sentinel/node/ClusterNode.java | 34 +++++++++--------- .../csp/sentinel/node/DefaultNode.java | 29 ++++++++++++--- .../csp/sentinel/node/DefaultNodeBuilder.java | 2 ++ .../com/alibaba/csp/sentinel/node/Node.java | 35 ++++++++++++++++++- .../csp/sentinel/node/NodeBuilder.java | 12 +++++++ .../csp/sentinel/node/StatisticNode.java | 26 +++++++++----- .../csp/sentinel/node/metric/MetricNode.java | 6 ++++ .../node/metric/MetricTimerListener.java | 6 ++-- .../sentinel/node/metric/MetricsReader.java | 13 ++++--- .../slots/DefaultSlotChainBuilder.java | 2 +- .../controller/RateLimiterController.java | 12 +++---- .../clusterbuilder/ClusterBuilderSlot.java | 10 +++--- .../slots/statistic/base/LeapArray.java | 3 ++ ...ilder.java => ClusterNodeBuilderTest.java} | 12 ++++--- 14 files changed, 147 insertions(+), 55 deletions(-) rename sentinel-core/src/test/java/com/alibaba/csp/sentinel/slots/clusterbuilder/{ClusterNodeBuilder.java => ClusterNodeBuilderTest.java} (82%) diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/ClusterNode.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/ClusterNode.java index 45252de3..87edd805 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/ClusterNode.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/ClusterNode.java @@ -16,6 +16,7 @@ package com.alibaba.csp.sentinel.node; import java.util.HashMap; +import java.util.Map; import java.util.concurrent.locks.ReentrantLock; import com.alibaba.csp.sentinel.context.ContextUtil; @@ -31,7 +32,7 @@ import com.alibaba.csp.sentinel.slots.block.BlockException; * To distinguish invocation from different origin (declared in * {@link ContextUtil#enter(String name, String origin)}), * one {@link ClusterNode} holds an {@link #originCountMap}, this map holds {@link StatisticNode} - * of different origin. Use {@link #getOriginNode(String)} to get {@link Node} of the specific + * of different origin. Use {@link #getOrCreateOriginNode(String)} to get {@link Node} of the specific * origin.
* Note that 'origin' usually is Service Consumer's app name. *

@@ -42,30 +43,31 @@ import com.alibaba.csp.sentinel.slots.block.BlockException; public class ClusterNode extends StatisticNode { /** - *

- * the longer the application runs, the more stable this mapping will + * The longer the application runs, the more stable this mapping will * become. so we don't concurrent map but a lock. as this lock only happens - * at the very beginning while concurrent map will hold the lock all the - * time - *

+ * at the very beginning while concurrent map will hold the lock all the time. */ - private HashMap originCountMap = new HashMap(); - private ReentrantLock lock = new ReentrantLock(); + private Map originCountMap = new HashMap(); + + private final ReentrantLock lock = new ReentrantLock(); /** - * Get {@link Node} of the specific origin. Usually the origin is the Service Consumer's app name. + *

Get {@link Node} of the specific origin. Usually the origin is the Service Consumer's app name.

+ *

If the origin node for given origin is absent, then a new {@link StatisticNode} + * for the origin will be created and returned.

* - * @param origin The caller's name. It is declared in the + * @param origin The caller's name, which is designated in the {@code parameter} parameter * {@link ContextUtil#enter(String name, String origin)}. - * @return the {@link Node} of the specific origin. + * @return the {@link Node} of the specific origin */ - public Node getOriginNode(String origin) { + public Node getOrCreateOriginNode(String origin) { StatisticNode statisticNode = originCountMap.get(origin); if (statisticNode == null) { try { lock.lock(); statisticNode = originCountMap.get(origin); if (statisticNode == null) { + // The node is absent, create a new node for the origin. statisticNode = new StatisticNode(); HashMap newMap = new HashMap( originCountMap.size() + 1); @@ -80,15 +82,15 @@ public class ClusterNode extends StatisticNode { return statisticNode; } - public synchronized HashMap getOriginCountMap() { + public synchronized Map getOriginCountMap() { return originCountMap; } /** - * Add exception count only when {@code throwable} is not {@link BlockException#isBlockException(Throwable)} + * Add exception count only when given {@code throwable} is not a {@link BlockException}. * - * @param throwable - * @param count count to add. + * @param throwable target exception + * @param count count to add */ public void trace(Throwable throwable, int count) { if (!BlockException.isBlockException(throwable)) { diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/DefaultNode.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/DefaultNode.java index 521c7e60..628d11cc 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/DefaultNode.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/DefaultNode.java @@ -40,10 +40,19 @@ import com.alibaba.csp.sentinel.slots.nodeselector.NodeSelectorSlot; */ public class DefaultNode extends StatisticNode { + /** + * The resource associated with the node. + */ private ResourceWrapper id; - private volatile HashSet childList = new HashSet(); + /** + * The list of all child nodes. + */ + private volatile Set childList = new HashSet(); + /** + * Associated cluster node. + */ private ClusterNode clusterNode; public DefaultNode(ResourceWrapper id, ClusterNode clusterNode) { @@ -63,22 +72,32 @@ public class DefaultNode extends StatisticNode { this.clusterNode = clusterNode; } + /** + * Add child node to current node. + * + * @param node valid child node + */ public void addChild(Node node) { - + if (node == null) { + RecordLog.warn("Trying to add null child to node <{0}>, ignored", id.getName()); + return; + } if (!childList.contains(node)) { - synchronized (this) { if (!childList.contains(node)) { - HashSet newSet = new HashSet(childList.size() + 1); + Set newSet = new HashSet(childList.size() + 1); newSet.addAll(childList); newSet.add(node); childList = newSet; } } - RecordLog.info(String.format("Add child %s to %s", ((DefaultNode)node).id.getName(), id.getName())); + RecordLog.info("Add child <{0}> to node <{1}>", ((DefaultNode)node).id.getName(), id.getName()); } } + /** + * Reset the child node list. + */ public void removeChildList() { this.childList = new HashSet(); } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/DefaultNodeBuilder.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/DefaultNodeBuilder.java index a74dde3b..fffd0c99 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/DefaultNodeBuilder.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/DefaultNodeBuilder.java @@ -18,6 +18,8 @@ package com.alibaba.csp.sentinel.node; import com.alibaba.csp.sentinel.slotchain.ResourceWrapper; /** + * Default implementation of {@link NodeBuilder}. + * * @author qinan.qn */ public class DefaultNodeBuilder implements NodeBuilder { diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/Node.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/Node.java index ff82a41c..f836ae0b 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/Node.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/Node.java @@ -25,6 +25,7 @@ import com.alibaba.csp.sentinel.node.metric.MetricNode; * * @author qinan.qn * @author leyou + * @author Eric Zhao */ public interface Node { @@ -70,6 +71,11 @@ public interface Node { */ long successQps(); + /** + * Get estimated max success QPS till now. + * + * @return max success QPS + */ long maxSuccessQps(); /** @@ -79,9 +85,16 @@ public interface Node { /** * Get average rt per second. + * + * @return average response time per second */ long avgRt(); + /** + * Get minimal response time. + * + * @return recorded minimal response time + */ long minRt(); /** @@ -99,23 +112,43 @@ public interface Node { */ long previousPassQps(); + /** + * Fetch all valid metric nodes of resources. + * + * @return valid metric nodes of resources + */ Map metrics(); + /** + * Add pass count. + */ void addPassRequest(); /** * Add rt and success count. * - * @param rt + * @param rt response time */ void rt(long rt); + /** + * Increase the block count. + */ void increaseBlockQps(); + /** + * Increase the biz exception count. + */ void increaseExceptionQps(); + /** + * Increase current thread count. + */ void increaseThreadNum(); + /** + * Increase current thread count. + */ void decreaseThreadNum(); /** diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/NodeBuilder.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/NodeBuilder.java index 97262fca..872b5371 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/NodeBuilder.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/NodeBuilder.java @@ -24,7 +24,19 @@ import com.alibaba.csp.sentinel.slotchain.ResourceWrapper; */ public interface NodeBuilder { + /** + * Create a new {@link DefaultNode} as tree node. + * + * @param id resource + * @param clusterNode the cluster node of the provided resource + * @return new created tree node + */ DefaultNode buildTreeNode(ResourceWrapper id, ClusterNode clusterNode); + /** + * Create a new {@link ClusterNode} as universal statistic node for a single resource. + * + * @return new created cluster node + */ ClusterNode buildClusterNode(); } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/StatisticNode.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/StatisticNode.java index 43cc0039..e12c7750 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/StatisticNode.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/StatisticNode.java @@ -106,25 +106,24 @@ public class StatisticNode implements Node { */ private AtomicInteger curThreadNum = new AtomicInteger(0); + /** + * The last timestamp when metrics were fetched. + */ private long lastFetchTime = -1; @Override public Map metrics() { + // The fetch operation is thread-safe under a single-thread scheduler pool. long currentTime = TimeUtil.currentTimeMillis(); currentTime = currentTime - currentTime % 1000; Map metrics = new ConcurrentHashMap(); List nodesOfEverySecond = rollingCounterInMinute.details(); long newLastFetchTime = lastFetchTime; + // Iterate metrics of all resources, filter valid metrics (not-empty and up-to-date). for (MetricNode node : nodesOfEverySecond) { - if (node.getTimestamp() > lastFetchTime && node.getTimestamp() < currentTime) { - if (node.getPassQps() != 0 - || node.getBlockQps() != 0 - || node.getSuccessQps() != 0 - || node.getExceptionQps() != 0 - || node.getRt() != 0) { - metrics.put(node.getTimestamp(), node); - newLastFetchTime = Math.max(newLastFetchTime, node.getTimestamp()); - } + if (isNodeInTime(node, currentTime) && isValidMetricNode(node)) { + metrics.put(node.getTimestamp(), node); + newLastFetchTime = Math.max(newLastFetchTime, node.getTimestamp()); } } lastFetchTime = newLastFetchTime; @@ -132,6 +131,15 @@ public class StatisticNode implements Node { return metrics; } + private boolean isNodeInTime(MetricNode node, long currentTime) { + return node.getTimestamp() > lastFetchTime && node.getTimestamp() < currentTime; + } + + private boolean isValidMetricNode(MetricNode node) { + return node.getPassQps() > 0 || node.getBlockQps() > 0 || node.getSuccessQps() > 0 + || node.getExceptionQps() > 0 || node.getRt() > 0; + } + @Override public void reset() { rollingCounterInSecond = new ArrayMetric(1000 / SampleCountProperty.SAMPLE_COUNT, IntervalProperty.INTERVAL); diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricNode.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricNode.java index 682debf1..3f6970ee 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricNode.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricNode.java @@ -19,6 +19,12 @@ import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Date; +/** + * Metrics data for a specific resource at given {@code timestamp}. + * + * @author jialiang.linjl + * @author Carpenter Lee + */ public class MetricNode { private long timestamp; diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricTimerListener.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricTimerListener.java index 763bb951..a873a633 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricTimerListener.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricTimerListener.java @@ -27,6 +27,9 @@ import com.alibaba.csp.sentinel.node.ClusterNode; import com.alibaba.csp.sentinel.slotchain.ResourceWrapper; import com.alibaba.csp.sentinel.slots.clusterbuilder.ClusterBuilderSlot; +/** + * @author jialiang.linjl + */ public class MetricTimerListener implements Runnable { private static final MetricWriter metricWriter = new MetricWriter(SentinelConfig.singleMetricFileSize(), @@ -57,11 +60,10 @@ public class MetricTimerListener implements Runnable { try { metricWriter.write(entry.getKey(), entry.getValue()); } catch (Exception e) { - RecordLog.info("write metric error: ", e); + RecordLog.warn("[MetricTimerListener] Write metric error", e); } } } - } } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricsReader.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricsReader.java index cb82f244..07ae6a5f 100644 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricsReader.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/node/metric/MetricsReader.java @@ -22,12 +22,17 @@ import java.nio.charset.Charset; import java.util.ArrayList; import java.util.List; +/** + * Reads metrics data from log file. + */ class MetricsReader { + /** - * avoid OOM in any case + * Avoid OOM in any cases. */ - private static final int maxLinesReturn = 100000; - private Charset charset; + private static final int MAX_LINES_RETURN = 100000; + + private final Charset charset; public MetricsReader(Charset charset) { this.charset = charset; @@ -58,7 +63,7 @@ class MetricsReader { } else { return false; } - if (list.size() >= maxLinesReturn) { + if (list.size() >= MAX_LINES_RETURN) { return false; } } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java index ccb91226..9bf2596d 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/DefaultSlotChainBuilder.java @@ -28,7 +28,7 @@ import com.alibaba.csp.sentinel.slots.statistic.StatisticSlot; import com.alibaba.csp.sentinel.slots.system.SystemSlot; /** - * Helper class to create {@link ProcessorSlotChain}. + * Builder for a default {@link ProcessorSlotChain}. * * @author qinan.qn * @author leyou diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/controller/RateLimiterController.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/controller/RateLimiterController.java index 4f0a868c..f1dc74af 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/controller/RateLimiterController.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/controller/RateLimiterController.java @@ -29,6 +29,7 @@ public class RateLimiterController implements TrafficShapingController { private final int maxQueueingTimeMs; private final double count; + private final AtomicLong latestPassedTime = new AtomicLong(-1); public RateLimiterController(int timeOut, double count) { @@ -38,21 +39,19 @@ public class RateLimiterController implements TrafficShapingController { @Override public boolean canPass(Node node, int acquireCount) { - - // 按照斜率来计算计划中应该什么时候通过 long currentTime = TimeUtil.currentTimeMillis(); - + // Calculate the interval between every two requests. long costTime = Math.round(1.0 * (acquireCount) / count * 1000); - //期待时间 + // Expected pass time of this request. long expectedTime = costTime + latestPassedTime.get(); if (expectedTime <= currentTime) { - //这里会有冲突,然而冲突就冲突吧. + // Contention may exist here, but it's okay. latestPassedTime.set(currentTime); return true; } else { - // 计算自己需要的等待时间 + // Calculate the time to wait. long waitTime = costTime + latestPassedTime.get() - TimeUtil.currentTimeMillis(); if (waitTime >= maxQueueingTimeMs) { return false; @@ -70,7 +69,6 @@ public class RateLimiterController implements TrafficShapingController { } } } - return false; } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/clusterbuilder/ClusterBuilderSlot.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/clusterbuilder/ClusterBuilderSlot.java index e5e91a15..f6faea0f 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/clusterbuilder/ClusterBuilderSlot.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/clusterbuilder/ClusterBuilderSlot.java @@ -51,7 +51,7 @@ public class ClusterBuilderSlot extends AbstractLinkedProcessorSlot *

* Remember that same resource({@link ResourceWrapper#equals(Object)}) will share * the same {@link ProcessorSlotChain} globally, no matter in witch context. So if - * code goes into {@link #entry(Context, ResourceWrapper, DefaultNode, int, Object...)}, + * code goes into {@link #entry(Context, ResourceWrapper, DefaultNode, int, boolean, Object...)}, * the resource name must be same but context name may not. *

*

@@ -62,8 +62,7 @@ public class ClusterBuilderSlot extends AbstractLinkedProcessorSlot *

* The longer the application runs, the more stable this mapping will * become. so we don't concurrent map but a lock. as this lock only happens - * at the very beginning while concurrent map will hold the lock all the - * time + * at the very beginning while concurrent map will hold the lock all the time. *

*/ private static volatile Map clusterNodeMap @@ -74,7 +73,8 @@ public class ClusterBuilderSlot extends AbstractLinkedProcessorSlot private ClusterNode clusterNode = null; @Override - public void entry(Context context, ResourceWrapper resourceWrapper, DefaultNode node, int count, boolean prioritized, Object... args) + public void entry(Context context, ResourceWrapper resourceWrapper, DefaultNode node, int count, + boolean prioritized, Object... args) throws Throwable { if (clusterNode == null) { synchronized (lock) { @@ -96,7 +96,7 @@ public class ClusterBuilderSlot extends AbstractLinkedProcessorSlot * the specific origin. */ if (!"".equals(context.getOrigin())) { - Node originNode = node.getClusterNode().getOriginNode(context.getOrigin()); + Node originNode = node.getClusterNode().getOrCreateOriginNode(context.getOrigin()); context.getCurEntry().setOriginNode(originNode); } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/statistic/base/LeapArray.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/statistic/base/LeapArray.java index 39ee3c04..f0357f7b 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/statistic/base/LeapArray.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/statistic/base/LeapArray.java @@ -45,6 +45,9 @@ public abstract class LeapArray { protected final AtomicReferenceArray> array; + /** + * The fine-grained update lock is used only when current bucket is deprecated. + */ private final ReentrantLock updateLock = new ReentrantLock(); /** diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/slots/clusterbuilder/ClusterNodeBuilder.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/slots/clusterbuilder/ClusterNodeBuilderTest.java similarity index 82% rename from sentinel-core/src/test/java/com/alibaba/csp/sentinel/slots/clusterbuilder/ClusterNodeBuilder.java rename to sentinel-core/src/test/java/com/alibaba/csp/sentinel/slots/clusterbuilder/ClusterNodeBuilderTest.java index baa9530a..eb595faf 100755 --- a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/slots/clusterbuilder/ClusterNodeBuilder.java +++ b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/slots/clusterbuilder/ClusterNodeBuilderTest.java @@ -15,6 +15,8 @@ */ package com.alibaba.csp.sentinel.slots.clusterbuilder; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import org.junit.Test; @@ -28,7 +30,7 @@ import com.alibaba.csp.sentinel.node.Node; /** * @author jialiang.linjl */ -public class ClusterNodeBuilder { +public class ClusterNodeBuilderTest { @Test public void clusterNodeBuilder_normal() throws Exception { @@ -37,10 +39,10 @@ public class ClusterNodeBuilder { Entry nodeA = SphU.entry("nodeA"); Node curNode = nodeA.getCurNode(); - assertTrue(curNode.getClass() == DefaultNode.class); + assertSame(curNode.getClass(), DefaultNode.class); DefaultNode dN = (DefaultNode)curNode; assertTrue(dN.getClusterNode().getOriginCountMap().containsKey("caller1")); - assertTrue(nodeA.getOriginNode() == dN.getClusterNode().getOriginNode("caller1")); + assertSame(nodeA.getOriginNode(), dN.getClusterNode().getOrCreateOriginNode("caller1")); if (nodeA != null) { nodeA.exit(); @@ -52,10 +54,10 @@ public class ClusterNodeBuilder { nodeA = SphU.entry("nodeA"); curNode = nodeA.getCurNode(); - assertTrue(curNode.getClass() == DefaultNode.class); + assertSame(curNode.getClass(), DefaultNode.class); DefaultNode dN1 = (DefaultNode)curNode; assertTrue(dN1.getClusterNode().getOriginCountMap().containsKey("caller2")); - assertTrue(dN1 != dN); + assertNotSame(dN1, dN); if (nodeA != null) { nodeA.exit();