From 1741da0bab9e2c72512c0bcaae5b0f92388bf2eb Mon Sep 17 00:00:00 2001 From: Eric Zhao Date: Thu, 14 Mar 2019 14:05:13 +0800 Subject: [PATCH] Automatically de-duplicate rules when loading rules (#571) * De-duplicate rules automatically when loading rules * Update rule managers Signed-off-by: Eric Zhao --- .../block/authority/AuthorityRuleManager.java | 47 ++++---- .../slots/block/authority/AuthoritySlot.java | 6 +- .../block/degrade/DegradeRuleManager.java | 107 +++++++++++++----- .../slots/block/flow/FlowRuleManager.java | 2 + .../slots/block/flow/FlowRuleUtil.java | 24 ++-- .../flow/param/ParamFlowRuleManager.java | 35 +++--- 6 files changed, 138 insertions(+), 83 deletions(-) diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/authority/AuthorityRuleManager.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/authority/AuthorityRuleManager.java index da72a226..7962c50e 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/authority/AuthorityRuleManager.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/authority/AuthorityRuleManager.java @@ -16,12 +16,15 @@ package com.alibaba.csp.sentinel.slots.block.authority; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import com.alibaba.csp.sentinel.log.RecordLog; import com.alibaba.csp.sentinel.slots.block.RuleConstant; +import com.alibaba.csp.sentinel.util.AssertUtil; import com.alibaba.csp.sentinel.util.StringUtil; import com.alibaba.csp.sentinel.property.DynamicSentinelProperty; import com.alibaba.csp.sentinel.property.PropertyListener; @@ -36,24 +39,22 @@ import com.alibaba.csp.sentinel.property.SentinelProperty; */ public final class AuthorityRuleManager { - private static Map> authorityRules - = new ConcurrentHashMap>(); + private static Map> authorityRules = new ConcurrentHashMap<>(); - final static RulePropertyListener listener = new RulePropertyListener(); - - private static SentinelProperty> currentProperty - = new DynamicSentinelProperty>(); + private static final RulePropertyListener LISTENER = new RulePropertyListener(); + private static SentinelProperty> currentProperty = new DynamicSentinelProperty<>(); static { - currentProperty.addListener(listener); + currentProperty.addListener(LISTENER); } public static void register2Property(SentinelProperty> property) { - synchronized (listener) { + AssertUtil.notNull(property, "property cannot be null"); + synchronized (LISTENER) { if (currentProperty != null) { - currentProperty.removeListener(listener); + currentProperty.removeListener(LISTENER); } - property.addListener(listener); + property.addListener(LISTENER); currentProperty = property; RecordLog.info("[AuthorityRuleManager] Registering new property to authority rule manager"); } @@ -78,11 +79,11 @@ public final class AuthorityRuleManager { * @return a new copy of the rules. */ public static List getRules() { - List rules = new ArrayList(); + List rules = new ArrayList<>(); if (authorityRules == null) { return rules; } - for (Map.Entry> entry : authorityRules.entrySet()) { + for (Map.Entry> entry : authorityRules.entrySet()) { rules.addAll(entry.getValue()); } return rules; @@ -92,7 +93,7 @@ public final class AuthorityRuleManager { @Override public void configUpdate(List conf) { - Map> rules = loadAuthorityConf(conf); + Map> rules = loadAuthorityConf(conf); authorityRules.clear(); if (rules != null) { @@ -101,8 +102,8 @@ public final class AuthorityRuleManager { RecordLog.info("[AuthorityRuleManager] Authority rules received: " + authorityRules); } - private Map> loadAuthorityConf(List list) { - Map> newRuleMap = new ConcurrentHashMap>(); + private Map> loadAuthorityConf(List list) { + Map> newRuleMap = new ConcurrentHashMap<>(); if (list == null || list.isEmpty()) { return newRuleMap; @@ -119,12 +120,12 @@ public final class AuthorityRuleManager { } String identity = rule.getResource(); - List ruleM = newRuleMap.get(identity); + Set ruleSet = newRuleMap.get(identity); // putIfAbsent - if (ruleM == null) { - ruleM = new ArrayList(); - ruleM.add(rule); - newRuleMap.put(identity, ruleM); + if (ruleSet == null) { + ruleSet = new HashSet<>(); + ruleSet.add(rule); + newRuleMap.put(identity, ruleSet); } else { // One resource should only have at most one authority rule, so just ignore redundant rules. RecordLog.warn("[AuthorityRuleManager] Ignoring redundant rule: " + rule.toString()); @@ -136,7 +137,7 @@ public final class AuthorityRuleManager { @Override public void configLoad(List value) { - Map> rules = loadAuthorityConf(value); + Map> rules = loadAuthorityConf(value); authorityRules.clear(); if (rules != null) { @@ -146,11 +147,11 @@ public final class AuthorityRuleManager { } } - static Map> getAuthorityRules() { + static Map> getAuthorityRules() { return authorityRules; } - static boolean isValidRule(AuthorityRule rule) { + public static boolean isValidRule(AuthorityRule rule) { return rule != null && !StringUtil.isBlank(rule.getResource()) && rule.getStrategy() >= 0 && StringUtil.isNotBlank(rule.getLimitApp()); } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/authority/AuthoritySlot.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/authority/AuthoritySlot.java index 369699c9..f7920baa 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/authority/AuthoritySlot.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/authority/AuthoritySlot.java @@ -15,8 +15,8 @@ */ package com.alibaba.csp.sentinel.slots.block.authority; -import java.util.List; import java.util.Map; +import java.util.Set; import com.alibaba.csp.sentinel.context.Context; import com.alibaba.csp.sentinel.node.DefaultNode; @@ -45,13 +45,13 @@ public class AuthoritySlot extends AbstractLinkedProcessorSlot { } void checkBlackWhiteAuthority(ResourceWrapper resource, Context context) throws AuthorityException { - Map> authorityRules = AuthorityRuleManager.getAuthorityRules(); + Map> authorityRules = AuthorityRuleManager.getAuthorityRules(); if (authorityRules == null) { return; } - List rules = authorityRules.get(resource.getName()); + Set rules = authorityRules.get(resource.getName()); if (rules == null) { return; } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeRuleManager.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeRuleManager.java index 5da818fa..9eb7e582 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeRuleManager.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeRuleManager.java @@ -16,10 +16,14 @@ package com.alibaba.csp.sentinel.slots.block.degrade; import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import com.alibaba.csp.sentinel.Constants; import com.alibaba.csp.sentinel.context.Context; import com.alibaba.csp.sentinel.log.RecordLog; import com.alibaba.csp.sentinel.node.DefaultNode; @@ -29,23 +33,24 @@ import com.alibaba.csp.sentinel.property.SentinelProperty; import com.alibaba.csp.sentinel.slotchain.ResourceWrapper; import com.alibaba.csp.sentinel.slots.block.BlockException; import com.alibaba.csp.sentinel.slots.block.RuleConstant; +import com.alibaba.csp.sentinel.util.AssertUtil; import com.alibaba.csp.sentinel.util.StringUtil; -/*** +/** * @author youji.zj * @author jialiang.linjl + * @author Eric Zhao */ -public class DegradeRuleManager { +public final class DegradeRuleManager { - private static volatile Map> degradeRules - = new ConcurrentHashMap>(); + private static final Map> degradeRules = new ConcurrentHashMap<>(); - final static RulePropertyListener listener = new RulePropertyListener(); + private static final RulePropertyListener LISTENER = new RulePropertyListener(); private static SentinelProperty> currentProperty - = new DynamicSentinelProperty>(); + = new DynamicSentinelProperty<>(); static { - currentProperty.addListener(listener); + currentProperty.addListener(LISTENER); } /** @@ -55,21 +60,19 @@ public class DegradeRuleManager { * @param property the property to listen. */ public static void register2Property(SentinelProperty> property) { - synchronized (listener) { + AssertUtil.notNull(property, "property cannot be null"); + synchronized (LISTENER) { RecordLog.info("[DegradeRuleManager] Registering new property to degrade rule manager"); - currentProperty.removeListener(listener); - property.addListener(listener); + currentProperty.removeListener(LISTENER); + property.addListener(LISTENER); currentProperty = property; } } public static void checkDegrade(ResourceWrapper resource, Context context, DefaultNode node, int count) throws BlockException { - if (degradeRules == null) { - return; - } - List rules = degradeRules.get(resource.getName()); + Set rules = degradeRules.get(resource.getName()); if (rules == null) { return; } @@ -82,6 +85,9 @@ public class DegradeRuleManager { } public static boolean hasConfig(String resource) { + if (resource == null) { + return false; + } return degradeRules.containsKey(resource); } @@ -91,11 +97,8 @@ public class DegradeRuleManager { * @return a new copy of the rules. */ public static List getRules() { - List rules = new ArrayList(); - if (degradeRules == null) { - return rules; - } - for (Map.Entry> entry : degradeRules.entrySet()) { + List rules = new ArrayList<>(); + for (Map.Entry> entry : degradeRules.entrySet()) { rules.addAll(entry.getValue()); } return rules; @@ -110,7 +113,42 @@ public class DegradeRuleManager { try { currentProperty.updateValue(rules); } catch (Throwable e) { - RecordLog.info(e.getMessage(), e); + RecordLog.warn("[DegradeRuleManager] Unexpected error when loading degrade rules", e); + } + } + + /** + * Set degrade rules for provided resource. Former rules of the resource will be replaced. + * + * @param resourceName valid resource name + * @param rules new rule set to load + * @return whether the rules has actually been updated + * @since 1.5.0 + */ + public static boolean setRulesForResource(String resourceName, Set rules) { + AssertUtil.notEmpty(resourceName, "resourceName cannot be empty"); + try { + Map> newRuleMap = new HashMap<>(degradeRules); + if (rules == null) { + newRuleMap.remove(resourceName); + } else { + Set newSet = new HashSet<>(); + for (DegradeRule rule : rules) { + if (isValidRule(rule) && resourceName.equals(rule.getResource())) { + newSet.add(rule); + } + } + newRuleMap.put(resourceName, newSet); + } + List allRules = new ArrayList<>(); + for (Set set : newRuleMap.values()) { + allRules.addAll(set); + } + return currentProperty.updateValue(allRules); + } catch (Throwable e) { + RecordLog.warn( + "[DegradeRuleManager] Unexpected error when setting degrade rules for resource: " + resourceName, e); + return false; } } @@ -118,7 +156,7 @@ public class DegradeRuleManager { @Override public void configUpdate(List conf) { - Map> rules = loadDegradeConf(conf); + Map> rules = loadDegradeConf(conf); if (rules != null) { degradeRules.clear(); degradeRules.putAll(rules); @@ -128,7 +166,7 @@ public class DegradeRuleManager { @Override public void configLoad(List conf) { - Map> rules = loadDegradeConf(conf); + Map> rules = loadDegradeConf(conf); if (rules != null) { degradeRules.clear(); degradeRules.putAll(rules); @@ -136,8 +174,8 @@ public class DegradeRuleManager { RecordLog.info("[DegradeRuleManager] Degrade rules loaded: " + degradeRules); } - private Map> loadDegradeConf(List list) { - Map> newRuleMap = new ConcurrentHashMap>(); + private Map> loadDegradeConf(List list) { + Map> newRuleMap = new ConcurrentHashMap<>(); if (list == null || list.isEmpty()) { return newRuleMap; @@ -145,7 +183,8 @@ public class DegradeRuleManager { for (DegradeRule rule : list) { if (!isValidRule(rule)) { - RecordLog.warn("[DegradeRuleManager] Ignoring invalid degrade rule when loading new rules: " + rule); + RecordLog.warn( + "[DegradeRuleManager] Ignoring invalid degrade rule when loading new rules: " + rule); continue; } @@ -154,17 +193,16 @@ public class DegradeRuleManager { } String identity = rule.getResource(); - List ruleM = newRuleMap.get(identity); - if (ruleM == null) { - ruleM = new ArrayList(); - newRuleMap.put(identity, ruleM); + Set ruleSet = newRuleMap.get(identity); + if (ruleSet == null) { + ruleSet = new HashSet<>(); + newRuleMap.put(identity, ruleSet); } - ruleM.add(rule); + ruleSet.add(rule); } return newRuleMap; } - } public static boolean isValidRule(DegradeRule rule) { @@ -173,6 +211,13 @@ public class DegradeRuleManager { if (!baseValid) { return false; } + // Warn for RT mode that exceeds the {@code TIME_DROP_VALVE}. + int maxAllowedRt = Constants.TIME_DROP_VALVE; + if (rule.getGrade() == RuleConstant.DEGRADE_GRADE_RT && rule.getCount() > maxAllowedRt) { + RecordLog.warn(String.format("[DegradeRuleManager] WARN: setting large RT threshold (%.1f ms) in RT mode" + + " will not take effect since it exceeds the max allowed value (%d ms)", rule.getCount(), + maxAllowedRt)); + } // Check exception ratio mode. if (rule.getGrade() == RuleConstant.DEGRADE_GRADE_EXCEPTION_RATIO && rule.getCount() > 1) { return false; diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleManager.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleManager.java index 9dd23026..18885812 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleManager.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleManager.java @@ -25,6 +25,7 @@ import java.util.concurrent.TimeUnit; import com.alibaba.csp.sentinel.concurrent.NamedThreadFactory; import com.alibaba.csp.sentinel.log.RecordLog; +import com.alibaba.csp.sentinel.util.AssertUtil; import com.alibaba.csp.sentinel.util.StringUtil; import com.alibaba.csp.sentinel.node.metric.MetricTimerListener; import com.alibaba.csp.sentinel.property.DynamicSentinelProperty; @@ -65,6 +66,7 @@ public class FlowRuleManager { * @param property the property to listen. */ public static void register2Property(SentinelProperty> property) { + AssertUtil.notNull(property, "property cannot be null"); synchronized (LISTENER) { RecordLog.info("[FlowRuleManager] Registering new property to flow rule manager"); currentProperty.removeListener(LISTENER); diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleUtil.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleUtil.java index 9d61229e..2c776d15 100644 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleUtil.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleUtil.java @@ -18,8 +18,11 @@ package com.alibaba.csp.sentinel.slots.block.flow; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import com.alibaba.csp.sentinel.log.RecordLog; @@ -85,10 +88,11 @@ public final class FlowRuleUtil { */ public static Map> buildFlowRuleMap(List list, Function groupFunction, Predicate filter, boolean shouldSort) { - Map> newRuleMap = new ConcurrentHashMap>(); + Map> newRuleMap = new ConcurrentHashMap<>(); if (list == null || list.isEmpty()) { return newRuleMap; } + Map> tmpMap = new ConcurrentHashMap<>(); for (FlowRule rule : list) { if (!isValidRule(rule)) { @@ -109,22 +113,24 @@ public final class FlowRuleUtil { if (key == null) { continue; } - List flowRules = newRuleMap.get(key); + Set flowRules = tmpMap.get(key); if (flowRules == null) { - flowRules = new ArrayList(); - newRuleMap.put(key, flowRules); + // Use hash set here to remove duplicate rules. + flowRules = new HashSet<>(); + tmpMap.put(key, flowRules); } flowRules.add(rule); } - - if (shouldSort && !newRuleMap.isEmpty()) { - Comparator comparator = new FlowRuleComparator(); - // Sort the rules. - for (List rules : newRuleMap.values()) { + Comparator comparator = new FlowRuleComparator(); + for (Entry> entries : tmpMap.entrySet()) { + List rules = new ArrayList<>(entries.getValue()); + if (shouldSort) { + // Sort the rules. Collections.sort(rules, comparator); } + newRuleMap.put(entries.getKey(), rules); } return newRuleMap; 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 dcd2c01f..40c923b5 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 @@ -16,6 +16,7 @@ package com.alibaba.csp.sentinel.slots.block.flow.param; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -26,6 +27,7 @@ import com.alibaba.csp.sentinel.property.DynamicSentinelProperty; import com.alibaba.csp.sentinel.property.PropertyListener; import com.alibaba.csp.sentinel.property.SentinelProperty; import com.alibaba.csp.sentinel.slots.block.RuleConstant; +import com.alibaba.csp.sentinel.util.AssertUtil; import com.alibaba.csp.sentinel.util.StringUtil; /** @@ -37,12 +39,10 @@ import com.alibaba.csp.sentinel.util.StringUtil; */ public final class ParamFlowRuleManager { - private static final Map> paramFlowRules - = new ConcurrentHashMap>(); + private static final Map> paramFlowRules = new ConcurrentHashMap<>(); private final static RulePropertyListener PROPERTY_LISTENER = new RulePropertyListener(); - private static SentinelProperty> currentProperty - = new DynamicSentinelProperty>(); + private static SentinelProperty> currentProperty = new DynamicSentinelProperty<>(); static { currentProperty.addListener(PROPERTY_LISTENER); @@ -68,6 +68,7 @@ public final class ParamFlowRuleManager { * @param property the property to listen */ public static void register2Property(SentinelProperty> property) { + AssertUtil.notNull(property, "property cannot be null"); synchronized (PROPERTY_LISTENER) { currentProperty.removeListener(PROPERTY_LISTENER); property.addListener(PROPERTY_LISTENER); @@ -77,11 +78,11 @@ public final class ParamFlowRuleManager { } public static List getRulesOfResource(String resourceName) { - return paramFlowRules.get(resourceName); + return new ArrayList<>(paramFlowRules.get(resourceName)); } public static boolean hasRules(String resourceName) { - List rules = paramFlowRules.get(resourceName); + Set rules = paramFlowRules.get(resourceName); return rules != null && !rules.isEmpty(); } @@ -91,8 +92,8 @@ public final class ParamFlowRuleManager { * @return a new copy of the rules. */ public static List getRules() { - List rules = new ArrayList(); - for (Map.Entry> entry : paramFlowRules.entrySet()) { + List rules = new ArrayList<>(); + for (Map.Entry> entry : paramFlowRules.entrySet()) { rules.addAll(entry.getValue()); } return rules; @@ -102,7 +103,7 @@ public final class ParamFlowRuleManager { @Override public void configUpdate(List list) { - Map> rules = aggregateHotParamRules(list); + Map> rules = aggregateHotParamRules(list); if (rules != null) { paramFlowRules.clear(); paramFlowRules.putAll(rules); @@ -112,7 +113,7 @@ public final class ParamFlowRuleManager { @Override public void configLoad(List list) { - Map> rules = aggregateHotParamRules(list); + Map> rules = aggregateHotParamRules(list); if (rules != null) { paramFlowRules.clear(); paramFlowRules.putAll(rules); @@ -120,8 +121,8 @@ public final class ParamFlowRuleManager { RecordLog.info("[ParamFlowRuleManager] Hot spot parameter flow rules received: " + paramFlowRules); } - private Map> aggregateHotParamRules(List list) { - Map> newRuleMap = new ConcurrentHashMap>(); + private Map> aggregateHotParamRules(List list) { + Map> newRuleMap = new ConcurrentHashMap<>(); if (list == null || list.isEmpty()) { // No parameter flow rules, so clear all the metrics. @@ -143,12 +144,12 @@ public final class ParamFlowRuleManager { ParamFlowRuleUtil.fillExceptionFlowItems(rule); String resourceName = rule.getResource(); - List ruleList = newRuleMap.get(resourceName); - if (ruleList == null) { - ruleList = new ArrayList(); - newRuleMap.put(resourceName, ruleList); + Set ruleSet = newRuleMap.get(resourceName); + if (ruleSet == null) { + ruleSet = new HashSet<>(); + newRuleMap.put(resourceName, ruleSet); } - ruleList.add(rule); + ruleSet.add(rule); } // Clear unused hot param metrics.