From 4dee63530ca7502792e073a989c66a0e8fc74e01 Mon Sep 17 00:00:00 2001 From: leonliao Date: Sun, 11 Apr 2021 16:49:27 +0800 Subject: [PATCH] [Dubbo-4140] Fix the StatItem.isAllowable() always lets 1 more request pass (#4141) * [Dubbo-4140] Fix the StatItem.isAllowable() always lets 1 more request pass for a period * [Dubbo-4140] Fix the related test case for fixing (#4101). Co-authored-by: Xin Wang --- .../apache/dubbo/rpc/filter/tps/StatItem.java | 2 +- .../rpc/filter/tps/DefaultTPSLimiterTest.java | 22 +++++++++++-------- .../dubbo/rpc/filter/tps/StatItemTest.java | 11 ++++++++++ 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/tps/StatItem.java b/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/tps/StatItem.java index 9fb3fab7d91..348b9ccef50 100644 --- a/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/tps/StatItem.java +++ b/dubbo-rpc/dubbo-rpc-api/src/main/java/org/apache/dubbo/rpc/filter/tps/StatItem.java @@ -49,7 +49,7 @@ public boolean isAllowable() { lastResetTime = now; } - if (token.sum() < 0) { + if (token.sum() <= 0) { return false; } token.decrement(); diff --git a/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/tps/DefaultTPSLimiterTest.java b/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/tps/DefaultTPSLimiterTest.java index 6633f587a9c..53969affc40 100644 --- a/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/tps/DefaultTPSLimiterTest.java +++ b/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/tps/DefaultTPSLimiterTest.java @@ -29,6 +29,7 @@ public class DefaultTPSLimiterTest { + private static final int TEST_LIMIT_RATE = 2; private DefaultTPSLimiter defaultTPSLimiter = new DefaultTPSLimiter(); @Test @@ -36,9 +37,9 @@ public void testIsAllowable() throws Exception { Invocation invocation = new MockInvocation(); URL url = URL.valueOf("test://test"); url = url.addParameter(INTERFACE_KEY, "org.apache.dubbo.rpc.file.TpsService"); - url = url.addParameter(TPS_LIMIT_RATE_KEY, 2); + url = url.addParameter(TPS_LIMIT_RATE_KEY, TEST_LIMIT_RATE); url = url.addParameter(TPS_LIMIT_INTERVAL_KEY, 1000); - for (int i = 0; i < 3; i++) { + for (int i = 1; i <= TEST_LIMIT_RATE; i++) { Assertions.assertTrue(defaultTPSLimiter.isAllowable(url, invocation)); } } @@ -48,10 +49,10 @@ public void testIsNotAllowable() throws Exception { Invocation invocation = new MockInvocation(); URL url = URL.valueOf("test://test"); url = url.addParameter(INTERFACE_KEY, "org.apache.dubbo.rpc.file.TpsService"); - url = url.addParameter(TPS_LIMIT_RATE_KEY, 2); + url = url.addParameter(TPS_LIMIT_RATE_KEY, TEST_LIMIT_RATE); url = url.addParameter(TPS_LIMIT_INTERVAL_KEY, 1000); - for (int i = 0; i < 4; i++) { - if (i == 3) { + for (int i = 1; i <= TEST_LIMIT_RATE + 1; i++) { + if (i == TEST_LIMIT_RATE + 1) { Assertions.assertFalse(defaultTPSLimiter.isAllowable(url, invocation)); } else { Assertions.assertTrue(defaultTPSLimiter.isAllowable(url, invocation)); @@ -65,14 +66,17 @@ public void testConfigChange() throws Exception { Invocation invocation = new MockInvocation(); URL url = URL.valueOf("test://test"); url = url.addParameter(INTERFACE_KEY, "org.apache.dubbo.rpc.file.TpsService"); - url = url.addParameter(TPS_LIMIT_RATE_KEY, 2); + url = url.addParameter(TPS_LIMIT_RATE_KEY, TEST_LIMIT_RATE); url = url.addParameter(TPS_LIMIT_INTERVAL_KEY, 1000); - for (int i = 0; i < 3; i++) { + for (int i = 1; i <= TEST_LIMIT_RATE; i++) { Assertions.assertTrue(defaultTPSLimiter.isAllowable(url, invocation)); } - url = url.addParameter(TPS_LIMIT_RATE_KEY, 2000); - for (int i = 0; i < 3; i++) { + final int tenTimesLimitRate = TEST_LIMIT_RATE * 10; + url = url.addParameter(TPS_LIMIT_RATE_KEY, tenTimesLimitRate); + for (int i = 1; i <= tenTimesLimitRate; i++) { Assertions.assertTrue(defaultTPSLimiter.isAllowable(url, invocation)); } + + Assertions.assertFalse(defaultTPSLimiter.isAllowable(url, invocation)); } } diff --git a/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/tps/StatItemTest.java b/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/tps/StatItemTest.java index 78c0c9cb0ec..e8c6bf80233 100644 --- a/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/tps/StatItemTest.java +++ b/dubbo-rpc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/tps/StatItemTest.java @@ -42,4 +42,15 @@ public void testIsAllowable() throws Exception { assertEquals(4, statItem.getToken()); } + @Test + public void testAccuracy() throws Exception { + final int EXPECTED_RATE = 5; + statItem = new StatItem("test", EXPECTED_RATE, 60_000L); + for (int i = 1; i <= EXPECTED_RATE; i++) { + assertEquals(true, statItem.isAllowable()); + } + + // Must block the 6th item + assertEquals(false, statItem.isAllowable()); + } }