From 096a9ebf7d53688480f9996f0e5ec591e73eb42d Mon Sep 17 00:00:00 2001 From: wavesZh Date: Thu, 7 May 2020 20:04:12 +0800 Subject: [PATCH] Improve deprecated ParameterMetric purge mechanism (#1372) * Clear useless data in ParameterMetric for all removed rules --- .../common/rule/GatewayRuleManager.java | 11 +++++-- .../flow/param/ParamFlowRuleManager.java | 32 +++++++++++-------- .../block/flow/param/ParameterMetric.java | 8 +++++ .../flow/param/ParamFlowRuleManagerTest.java | 30 +++++++++++++++++ 4 files changed, 66 insertions(+), 15 deletions(-) diff --git a/sentinel-adapter/sentinel-api-gateway-adapter-common/src/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/rule/GatewayRuleManager.java b/sentinel-adapter/sentinel-api-gateway-adapter-common/src/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/rule/GatewayRuleManager.java index 8744707d..257b5c03 100644 --- a/sentinel-adapter/sentinel-api-gateway-adapter-common/src/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/rule/GatewayRuleManager.java +++ b/sentinel-adapter/sentinel-api-gateway-adapter-common/src/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/rule/GatewayRuleManager.java @@ -217,10 +217,17 @@ public final class GatewayRuleManager { } // Clear unused parameter metrics. - Set previousResources = CONVERTED_PARAM_RULE_MAP.keySet(); - for (String resource : previousResources) { + for (Map.Entry> entry : CONVERTED_PARAM_RULE_MAP.entrySet()) { + String resource = entry.getKey(); if (!newRuleMap.containsKey(resource)) { ParameterMetricStorage.clearParamMetricForResource(resource); + continue; + } + List newRuleList = newRuleMap.get(resource); + List oldRuleList = new ArrayList<>(entry.getValue()); + oldRuleList.removeAll(newRuleList); + for (ParamFlowRule rule : oldRuleList) { + ParameterMetricStorage.getParamMetricForResource(resource).clearForRule(rule); } } diff --git a/sentinel-extension/sentinel-parameter-flow-control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManager.java b/sentinel-extension/sentinel-parameter-flow-control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManager.java index 2467ba64..b660e0d9 100644 --- a/sentinel-extension/sentinel-parameter-flow-control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManager.java +++ b/sentinel-extension/sentinel-parameter-flow-control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManager.java @@ -18,7 +18,6 @@ package com.alibaba.csp.sentinel.slots.block.flow.param; import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import com.alibaba.csp.sentinel.log.RecordLog; @@ -36,7 +35,7 @@ import com.alibaba.csp.sentinel.util.AssertUtil; */ public final class ParamFlowRuleManager { - private static final Map> paramFlowRules = new ConcurrentHashMap<>(); + private static final Map> PARAM_FLOW_RULES = new ConcurrentHashMap<>(); private final static RulePropertyListener PROPERTY_LISTENER = new RulePropertyListener(); private static SentinelProperty> currentProperty = new DynamicSentinelProperty<>(); @@ -76,11 +75,11 @@ public final class ParamFlowRuleManager { } public static List getRulesOfResource(String resourceName) { - return new ArrayList<>(paramFlowRules.get(resourceName)); + return new ArrayList<>(PARAM_FLOW_RULES.get(resourceName)); } public static boolean hasRules(String resourceName) { - List rules = paramFlowRules.get(resourceName); + List rules = PARAM_FLOW_RULES.get(resourceName); return rules != null && !rules.isEmpty(); } @@ -91,7 +90,7 @@ public final class ParamFlowRuleManager { */ public static List getRules() { List rules = new ArrayList<>(); - for (Map.Entry> entry : paramFlowRules.entrySet()) { + for (Map.Entry> entry : PARAM_FLOW_RULES.entrySet()) { rules.addAll(entry.getValue()); } return rules; @@ -103,20 +102,20 @@ public final class ParamFlowRuleManager { public void configUpdate(List list) { Map> rules = aggregateAndPrepareParamRules(list); if (rules != null) { - paramFlowRules.clear(); - paramFlowRules.putAll(rules); + PARAM_FLOW_RULES.clear(); + PARAM_FLOW_RULES.putAll(rules); } - RecordLog.info("[ParamFlowRuleManager] Parameter flow rules received: " + paramFlowRules); + RecordLog.info("[ParamFlowRuleManager] Parameter flow rules received: " + PARAM_FLOW_RULES); } @Override public void configLoad(List list) { Map> rules = aggregateAndPrepareParamRules(list); if (rules != null) { - paramFlowRules.clear(); - paramFlowRules.putAll(rules); + PARAM_FLOW_RULES.clear(); + PARAM_FLOW_RULES.putAll(rules); } - RecordLog.info("[ParamFlowRuleManager] Parameter flow rules received: " + paramFlowRules); + RecordLog.info("[ParamFlowRuleManager] Parameter flow rules received: " + PARAM_FLOW_RULES); } private Map> aggregateAndPrepareParamRules(List list) { @@ -129,10 +128,17 @@ public final class ParamFlowRuleManager { } // Clear unused parameter metrics. - Set previousResources = paramFlowRules.keySet(); - for (String resource : previousResources) { + for (Map.Entry> entry : PARAM_FLOW_RULES.entrySet()) { + String resource = entry.getKey(); if (!newRuleMap.containsKey(resource)) { ParameterMetricStorage.clearParamMetricForResource(resource); + continue; + } + List newRuleList = newRuleMap.get(resource); + List oldRuleList = new ArrayList<>(entry.getValue()); + oldRuleList.removeAll(newRuleList); + for (ParamFlowRule rule : oldRuleList) { + ParameterMetricStorage.getParamMetricForResource(resource).clearForRule(rule); } } diff --git a/sentinel-extension/sentinel-parameter-flow-control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetric.java b/sentinel-extension/sentinel-parameter-flow-control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetric.java index daa436f3..ee29728a 100644 --- a/sentinel-extension/sentinel-parameter-flow-control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetric.java +++ b/sentinel-extension/sentinel-parameter-flow-control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetric.java @@ -84,6 +84,14 @@ public class ParameterMetric { } } + public void clearForRule(ParamFlowRule rule) { + synchronized (lock) { + ruleTimeCounters.remove(rule); + ruleTokenCounter.remove(rule); + threadCountMap.remove(rule.getParamIdx()); + } + } + public void initialize(ParamFlowRule rule) { if (!ruleTimeCounters.containsKey(rule)) { synchronized (lock) { diff --git a/sentinel-extension/sentinel-parameter-flow-control/src/test/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManagerTest.java b/sentinel-extension/sentinel-parameter-flow-control/src/test/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManagerTest.java index 8241517b..974372b3 100644 --- a/sentinel-extension/sentinel-parameter-flow-control/src/test/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManagerTest.java +++ b/sentinel-extension/sentinel-parameter-flow-control/src/test/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManagerTest.java @@ -66,6 +66,36 @@ public class ParamFlowRuleManagerTest { ParameterMetricStorage.getParamMetricForResource(resA)); } + @Test + public void testLoadParamRulesClearingUnusedMetricsForRule() { + final String resA = "resA"; + ParamFlowRule ruleA1 = new ParamFlowRule(resA) + .setCount(1) + .setParamIdx(0); + ParamFlowRule ruleA2 = new ParamFlowRule(resA) + .setCount(2) + .setParamIdx(1); + + ParamFlowRuleManager.loadRules(Arrays.asList(ruleA1, ruleA2)); + ParameterMetric metric = new ParameterMetric(); + metric.initialize(ruleA1); + metric.initialize(ruleA2); + ParameterMetricStorage.getMetricsMap().put(resA, metric); + + ParameterMetric metric1 = ParameterMetricStorage.getParamMetricForResource(resA); + assertNotNull(metric1); + assertNotNull(metric1.getRuleTimeCounter(ruleA1)); + assertNotNull(metric1.getRuleTimeCounter(ruleA2)); + + ParamFlowRuleManager.loadRules(Arrays.asList(ruleA1)); + + ParameterMetric metric2 = ParameterMetricStorage.getParamMetricForResource(resA); + assertNotNull(metric2); + assertNotNull(metric2.getRuleTimeCounter(ruleA1)); + assertNull(metric2.getRuleTimeCounter(ruleA2)); + } + + @Test public void testLoadParamRulesAndGet() { final String resA = "abc";