[Dubbo-4140] Fix the StatItem.isAllowable() always lets 1 more reques…

…t 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 <[email protected]>
apache · Apr 11, 2021 · 4dee635 · 4dee635
1 parent 27fbe77
commit 4dee635
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 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
@@ -49,7 +49,7 @@ public boolean isAllowable() {
             lastResetTime = now;
         }
 
-        if (token.sum() < 0) {
+        if (token.sum() <= 0) {
             return false;
         }
         token.decrement();

diff --git a/...pc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/tps/DefaultTPSLimiterTest.java b/...pc/dubbo-rpc-api/src/test/java/org/apache/dubbo/rpc/filter/tps/DefaultTPSLimiterTest.java
@@ -29,16 +29,17 @@
 
 public class DefaultTPSLimiterTest {
 
+    private static final int TEST_LIMIT_RATE = 2;
     private DefaultTPSLimiter defaultTPSLimiter = new DefaultTPSLimiter();
 
     @Test
     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
@@ -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());
+	}
 }