Improve deprecated ParameterMetric purge mechanism (#1372)

* Clear useless data in ParameterMetric for all removed rules
alibaba · May 7, 2020 · 096a9eb · 096a9eb
1 parent d84d681
commit 096a9eb
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 15 deletions.
diff --git a/...rc/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/rule/GatewayRuleManager.java b/...rc/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/rule/GatewayRuleManager.java
@@ -217,10 +217,17 @@ private void applyToConvertedParamMap(Set<ParamFlowRule> paramFlowRules) {
             }
 
             // Clear unused parameter metrics.
-            Set<String> previousResources = CONVERTED_PARAM_RULE_MAP.keySet();
-            for (String resource : previousResources) {
+            for (Map.Entry<String, List<ParamFlowRule>> entry : CONVERTED_PARAM_RULE_MAP.entrySet()) {
+                String resource = entry.getKey();
                 if (!newRuleMap.containsKey(resource)) {
                     ParameterMetricStorage.clearParamMetricForResource(resource);
+                    continue;
+                }
+                List<ParamFlowRule> newRuleList = newRuleMap.get(resource);
+                List<ParamFlowRule> oldRuleList = new ArrayList<>(entry.getValue());
+                oldRuleList.removeAll(newRuleList);
+                for (ParamFlowRule rule : oldRuleList) {
+                    ParameterMetricStorage.getParamMetricForResource(resource).clearForRule(rule);
                 }
             }
 

diff --git a/...l/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManager.java b/...l/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManager.java
@@ -18,7 +18,6 @@
 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 @@
  */
 public final class ParamFlowRuleManager {
 
-    private static final Map<String, List<ParamFlowRule>> paramFlowRules = new ConcurrentHashMap<>();
+    private static final Map<String, List<ParamFlowRule>> PARAM_FLOW_RULES = new ConcurrentHashMap<>();
 
     private final static RulePropertyListener PROPERTY_LISTENER = new RulePropertyListener();
     private static SentinelProperty<List<ParamFlowRule>> currentProperty = new DynamicSentinelProperty<>();
@@ -76,11 +75,11 @@ public static void register2Property(SentinelProperty<List<ParamFlowRule>> prope
     }
 
     public static List<ParamFlowRule> getRulesOfResource(String resourceName) {
-        return new ArrayList<>(paramFlowRules.get(resourceName));
+        return new ArrayList<>(PARAM_FLOW_RULES.get(resourceName));
     }
 
     public static boolean hasRules(String resourceName) {
-        List<ParamFlowRule> rules = paramFlowRules.get(resourceName);
+        List<ParamFlowRule> rules = PARAM_FLOW_RULES.get(resourceName);
         return rules != null && !rules.isEmpty();
     }
 
@@ -91,7 +90,7 @@ public static boolean hasRules(String resourceName) {
      */
     public static List<ParamFlowRule> getRules() {
         List<ParamFlowRule> rules = new ArrayList<>();
-        for (Map.Entry<String, List<ParamFlowRule>> entry : paramFlowRules.entrySet()) {
+        for (Map.Entry<String, List<ParamFlowRule>> entry : PARAM_FLOW_RULES.entrySet()) {
             rules.addAll(entry.getValue());
         }
         return rules;
@@ -103,20 +102,20 @@ static class RulePropertyListener implements PropertyListener<List<ParamFlowRule
         public void configUpdate(List<ParamFlowRule> list) {
             Map<String, List<ParamFlowRule>> 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<ParamFlowRule> list) {
             Map<String, List<ParamFlowRule>> 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<String, List<ParamFlowRule>> aggregateAndPrepareParamRules(List<ParamFlowRule> list) {
@@ -129,10 +128,17 @@ private Map<String, List<ParamFlowRule>> aggregateAndPrepareParamRules(List<Para
             }
 
             // Clear unused parameter metrics.
-            Set<String> previousResources = paramFlowRules.keySet();
-            for (String resource : previousResources) {
+            for (Map.Entry<String, List<ParamFlowRule>> entry : PARAM_FLOW_RULES.entrySet()) {
+                String resource = entry.getKey();
                 if (!newRuleMap.containsKey(resource)) {
                     ParameterMetricStorage.clearParamMetricForResource(resource);
+                    continue;
+                }
+                List<ParamFlowRule> newRuleList = newRuleMap.get(resource);
+                List<ParamFlowRule> oldRuleList = new ArrayList<>(entry.getValue());
+                oldRuleList.removeAll(newRuleList);
+                for (ParamFlowRule rule : oldRuleList) {
+                    ParameterMetricStorage.getParamMetricForResource(resource).clearForRule(rule);
                 }
             }
 

diff --git a/...ontrol/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetric.java b/...ontrol/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetric.java
@@ -84,6 +84,14 @@ public void clear() {
         }
     }
 
+    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/...c/test/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManagerTest.java b/...c/test/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleManagerTest.java
@@ -66,6 +66,36 @@ public void testLoadParamRulesClearingUnusedMetrics() {
             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";