From 04e791b153fbac95896a25d872a78d174e6b8e64 Mon Sep 17 00:00:00 2001 From: hutiefang76 <137664623+hutiefang76@users.noreply.github.com> Date: Wed, 7 Oct 2026 16:23:48 +0800 Subject: [PATCH] Preserve shared parameter thread counters when removing a rule --- .../block/flow/param/ParameterMetric.java | 24 +++++++- .../flow/param/ParamFlowRuleManagerTest.java | 41 +++++++++++++ .../block/flow/param/ParameterMetricTest.java | 59 +++++++++++++++++++ 3 files changed, 123 insertions(+), 1 deletion(-) 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 a5ab902c27..7c0f3b5b27 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 @@ -19,6 +19,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -91,8 +92,29 @@ public void clearForRule(ParamFlowRule rule) { synchronized (lock) { ruleTimeCounters.remove(rule); ruleTokenCounter.remove(rule); - threadCountMap.remove(rule.getParamIdx()); + // The thread counters are shared by all the rules with the same parameter index, + // so the counter of the index should only be removed when no other rule still + // relies on it. Otherwise the in-flight thread count of the surviving rules + // would be dropped, making them under-counted. + Integer paramIdx = rule.getParamIdx(); + if (!isParamIdxInUse(paramIdx)) { + threadCountMap.remove(paramIdx); + } + } + } + + private boolean isParamIdxInUse(Integer paramIdx) { + for (ParamFlowRule rule : ruleTimeCounters.keySet()) { + if (Objects.equals(rule.getParamIdx(), paramIdx)) { + return true; + } + } + for (ParamFlowRule rule : ruleTokenCounter.keySet()) { + if (Objects.equals(rule.getParamIdx(), paramIdx)) { + return true; + } } + return false; } public void initialize(ParamFlowRule rule) { 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 fe21297d5a..2cb4f89fe3 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 @@ -96,6 +96,47 @@ public void testLoadParamRulesClearingUnusedMetricsForRule() { } + @Test + public void testLoadParamRulesKeepingSharedThreadCountForSurvivingRule() { + final String resA = "resA"; + final int sharedParamIdx = 0; + + // Two distinct rules share the same parameter index. + ParamFlowRule ruleA = new ParamFlowRule(resA) + .setCount(10) + .setParamIdx(sharedParamIdx) + .setGrade(RuleConstant.FLOW_GRADE_THREAD); + ParamFlowRule ruleB = new ParamFlowRule(resA) + .setCount(5) + .setParamIdx(sharedParamIdx); + + ParamFlowRuleManager.loadRules(Arrays.asList(ruleA, ruleB)); + + // Initialize the metrics for both rules, as ParameterMetricStorage#initParamMetricsFor does + // for the loaded rules. + ParameterMetric metric = new ParameterMetric(); + metric.initialize(ruleA); + metric.initialize(ruleB); + ParameterMetricStorage.getMetricsMap().put(resA, metric); + + // Seed a known thread count on the shared parameter index. + Object value = "v"; + metric.addThreadCount(value); + metric.addThreadCount(value); + long expectedThreadCount = metric.getThreadCount(sharedParamIdx, value); + assertTrue("Test fixture should have a positive thread count", expectedThreadCount > 0); + + // Reload with only rule A: rule B is removed while rule A survives on the same index. + ParamFlowRuleManager.loadRules(Collections.singletonList(ruleA)); + + ParameterMetric metricAfter = ParameterMetricStorage.getParamMetricForResource(resA); + assertNotNull(metricAfter); + assertEquals("The thread count of the surviving rule should be preserved", + expectedThreadCount, metricAfter.getThreadCount(sharedParamIdx, value)); + assertNotNull("The surviving rule's time counter should be preserved", metricAfter.getRuleTimeCounter(ruleA)); + assertNull("The removed rule's time counter should be dropped", metricAfter.getRuleTimeCounter(ruleB)); + } + @Test public void testLoadParamRulesAndGet() { final String resA = "abc"; diff --git a/sentinel-extension/sentinel-parameter-flow-control/src/test/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetricTest.java b/sentinel-extension/sentinel-parameter-flow-control/src/test/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetricTest.java index 8c7e7d8c64..824b13ae4d 100644 --- a/sentinel-extension/sentinel-parameter-flow-control/src/test/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetricTest.java +++ b/sentinel-extension/sentinel-parameter-flow-control/src/test/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParameterMetricTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -76,6 +77,64 @@ public void testInitAndClearParameterMetric() { assertEquals(0, metric.getRuleTokenCounterMap().size()); } + @Test + public void testClearForRuleKeepsSharedThreadCount() { + ParameterMetric metric = new ParameterMetric(); + + // Two distinct rules share the same parameter index. + ParamFlowRule ruleA = new ParamFlowRule("abc").setParamIdx(1); + ParamFlowRule ruleB = new ParamFlowRule("abc").setParamIdx(1).setCount(5); + metric.initialize(ruleA); + metric.initialize(ruleB); + + CacheMap threadCountMap = metric.getThreadCountMap().get(1); + assertNotNull(threadCountMap); + threadCountMap.put("v", new AtomicInteger(2)); + + // Clearing rule B should keep the shared thread counter, as rule A still uses index 1. + metric.clearForRule(ruleB); + assertNull("The removed rule's time counter should be cleared", metric.getRuleTimeCounter(ruleB)); + assertNull("The removed rule's token counter should be cleared", metric.getRuleTokenCounterMap().get(ruleB)); + assertNotNull("The surviving rule's time counter should be kept", metric.getRuleTimeCounter(ruleA)); + assertSame("The shared thread counter should be kept for the surviving rule", + threadCountMap, metric.getThreadCountMap().get(1)); + assertEquals(2L, metric.getThreadCount(1, "v")); + + // Once the last rule using index 1 is cleared, the shared counter should be removed. + metric.clearForRule(ruleA); + assertNull("The thread counter should be cleared when no rule uses the index", + metric.getThreadCountMap().get(1)); + assertEquals(0L, metric.getThreadCount(1, "v")); + } + + @Test + public void testClearForRuleRemovesThreadCountOfUnusedIndex() { + ParameterMetric metric = new ParameterMetric(); + + ParamFlowRule ruleA = new ParamFlowRule("abc").setParamIdx(1); + // Rule C uses a different, unique index. + ParamFlowRule ruleC = new ParamFlowRule("abc").setParamIdx(2); + metric.initialize(ruleA); + metric.initialize(ruleC); + + CacheMap sharedThreadCount = metric.getThreadCountMap().get(1); + CacheMap uniqueThreadCount = metric.getThreadCountMap().get(2); + assertNotNull(sharedThreadCount); + assertNotNull(uniqueThreadCount); + sharedThreadCount.put("v", new AtomicInteger(2)); + uniqueThreadCount.put("w", new AtomicInteger(3)); + + // Index 2 is only used by rule C, so its counter should be removed. + metric.clearForRule(ruleC); + assertNull("The thread counter of a unique index should be cleared", + metric.getThreadCountMap().get(2)); + assertEquals(0L, metric.getThreadCount(2, "w")); + // Index 1 is still used by rule A, so it should be untouched. + assertSame("The thread counter of an index still in use should be kept", + sharedThreadCount, metric.getThreadCountMap().get(1)); + assertEquals(2L, metric.getThreadCount(1, "v")); + } + @Test public void testAddAndDecreaseThreadCountCommon() { testAddAndDecreaseThreadCount(PARAM_TYPE_NORMAL);