Fix a bug in SmoothRateLimiter where getEvents will always return 0 (#…

…678)

CHANGELOG.md

@@ -14,6 +14,9 @@ and what APIs have changed, if applicable.
 
 ## [Unreleased]
 
+## [29.21.3] - 2021-08-25
+- Fix a bug in SmoothRateLimiter where getEvents will always return 0
+
 ## [29.21.2] - 2021-08-18
 - Remove support for disabling request validation via headers since doing so can have dangerous side effects.
 
@@ -5064,7 +5067,8 @@ patch operations can re-use these classes for generating patch messages.
 
 ## [0.14.1]
 
-[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.21.2...master
+[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.21.3...master
+[29.21.3]: https://github.com/linkedin/rest.li/compare/v29.21.2...v29.21.3
 [29.21.2]: https://github.com/linkedin/rest.li/compare/v29.21.1...v29.21.2
 [29.21.1]: https://github.com/linkedin/rest.li/compare/v29.21.0...v29.21.1
 [29.21.0]: https://github.com/linkedin/rest.li/compare/v29.20.1...v29.21.0

darkcluster/src/test/java/com/linkedin/darkcluster/TestConstantQpsDarkClusterStrategy.java

@@ -68,7 +68,8 @@ public Object[][] qpsKeys()
         {1000, 100, 40, 3},
         {1000, 200, 10, 1},
         {1000, 250, 10, 1},
-        {1000, 400, 10, 1}
+        {1000, 400, 10, 1},
+        {1000, 10, 400, 2}
     };
   }
 
@@ -104,7 +105,7 @@ public void testStrategy(int numIterations, int qps, int numSourceInstances, int
         strategy.handleRequest(dummyRestRequest, dummyRestRequest, new RequestContext());
       }
       executor.runFor(1000);
-      int expectedCount = ((numIterations == 0 ? 0 : 1) * qps * numDarkInstances)/(numSourceInstances);
+      int expectedCount = (int) Math.ceil(((numIterations == 0 ? 0 : 1) * qps * numDarkInstances)/ (double) (numSourceInstances));
       int actualCount = baseDispatcher.getRequestCount();
       Assert.assertEquals(actualCount, expectedCount, expectedCount * ERR_PCT, "count not within expected range");
     });

gradle.properties

@@ -1,4 +1,4 @@
-version=29.21.2
+version=29.21.3
 group=com.linkedin.pegasus
 org.gradle.configureondemand=true
 org.gradle.parallel=true

r2-core/src/main/java/com/linkedin/r2/transport/http/client/ratelimiter/Rate.java

@@ -62,10 +62,16 @@ public Rate(double events, double period, int burst)
       _period = newPeriod;
 
     }
-    else
-    {
-      _events = events;
-      _period = period;
+    else {
+      if (events > 0 && events < 1) {
+        _period = period / events;
+        _events = 1;
+      }
+      else
+      {
+        _events = events;
+        _period = period;
+      }
     }
   }
 

r2-core/src/test/java/com/linkedin/r2/transport/http/client/ratelimiter/TestConstantQpsRateLimiter.java

@@ -40,8 +40,9 @@ public class TestConstantQpsRateLimiter
   public void submitOnceGetMany()
   {
     ClockedExecutor executor = new ClockedExecutor();
+    ClockedExecutor circularBufferExecutor = new ClockedExecutor();
     ConstantQpsRateLimiter rateLimiter =
-        new ConstantQpsRateLimiter(executor, executor, executor, TestEvictingCircularBuffer.getBuffer(executor));
+        new ConstantQpsRateLimiter(executor, executor, executor, TestEvictingCircularBuffer.getBuffer(circularBufferExecutor));
 
     rateLimiter.setRate(TEST_QPS, ONE_SECOND, UNLIMITED_BURST);
     rateLimiter.setBufferCapacity(1);
@@ -52,6 +53,21 @@ public void submitOnceGetMany()
     Assert.assertTrue(tattler.getInteractCount() > 1);
   }
 
+  @Test(timeOut = TEST_TIMEOUT)
+  public void lowNonWholeRate()
+  {
+    ClockedExecutor executor = new ClockedExecutor();
+    ClockedExecutor circularBufferExecutor = new ClockedExecutor();
+    ConstantQpsRateLimiter rateLimiter =
+        new ConstantQpsRateLimiter(executor, executor, executor, TestEvictingCircularBuffer.getBuffer(circularBufferExecutor));
+    rateLimiter.setRate(0.05d, ONE_SECOND, UNLIMITED_BURST);
+    rateLimiter.setBufferCapacity(1);
+    TattlingCallback<None> tattler = new TattlingCallback<>();
+    rateLimiter.submit(tattler);
+    executor.runFor(59000);
+    Assert.assertTrue(tattler.getInteractCount() == 3);
+  }
+
   @Test(timeOut = TEST_TIMEOUT)
   public void eventLoopStopsWhenTtlExpiresAllRequests()
   {