Improve interface documentation and API around precision as this was …

…previously poorly designed and communicated. Will force breaking API change

ratelimitj-core/src/main/java/es/moki/ratelimitj/core/limiter/request/RequestLimitRule.java

@@ -38,34 +38,38 @@ private RequestLimitRule(int durationSeconds, long limit, int precision, String
         this.keys = keys;
     }
 
+    private static void checkDuration(Duration duration) {
+        requireNonNull(duration, "duration can not be null");
+        if (Duration.ofSeconds(1).compareTo(duration) >= 1) {
+            throw new IllegalArgumentException("duration must be great than 1 second");
+        }
+    }
+
     /**
      * Initialise a request rate limit. Imagine the whole duration window as being one large bucket with a single count.
      *
-     * @param duration The time the limit will be applied over.
+     * @param duration The time the limit will be applied over. The duration must be greater than 1 second.
      * @param limit    A number representing the maximum operations that can be performed in the given duration.
      * @return A limit rule.
      */
     public static RequestLimitRule of(Duration duration, long limit) {
-        requireNonNull(duration, "duration can not be null");
+        checkDuration(duration);
         if (limit < 0) {
             throw new IllegalArgumentException("limit must be greater than zero.");
         }
-        if (Duration.ofSeconds(1).compareTo(duration) >= 1) {
-            throw new IllegalArgumentException("duration must be great than 1 second");
-        }
         int durationSeconds = (int) duration.getSeconds();
         return new RequestLimitRule(durationSeconds, limit, durationSeconds);
     }
 
     /**
-     * Configures as a sliding window rate limit. Imagine the duration window divided into a number of smaller buckets, each with it's own count.
-     * The number of smaller buckets is defined by the precision.
+     * Controls (approximate) sliding window precision. A lower duration increases precision and minimises the Thundering herd problem - https://en.wikipedia.org/wiki/Thundering_herd_problem
      *
-     * @param precision Defines the number of buckets that will be used to approximate the sliding window.
+     * @param precision Defines the time precision that will be used to approximate the sliding window. The precision must be greater than 1 second.
      * @return a limit rule
      */
-    public RequestLimitRule withPrecision(int precision) {
-        return new RequestLimitRule(this.durationSeconds, this.limit, precision, this.name, this.keys);
+    public RequestLimitRule withPrecision(Duration precision) {
+        checkDuration(precision);
+        return new RequestLimitRule(this.durationSeconds, this.limit, (int) precision.getSeconds(), this.name, this.keys);
     }
 
     /**
@@ -107,9 +111,9 @@ public int getDurationSeconds() {
     }
 
     /**
-     * @return The limits precision.
+     * @return The limits precision in seconds.
      */
-    public int getPrecision() {
+    public int getPrecisionSeconds() {
         return precision;
     }
 

ratelimitj-core/src/test/java/es/moki/ratelimitj/core/limiter/request/RequestLimitRuleTest.java

@@ -3,7 +3,6 @@
 import org.junit.jupiter.api.Test;
 
 import java.time.Duration;
-import java.util.concurrent.TimeUnit;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -29,7 +28,7 @@ void shouldHaveDuration60Seconds() {
     void shouldDefaultPrecisionToEqualDuration() {
         RequestLimitRule requestLimitRule = RequestLimitRule.of(Duration.ofMinutes(1), 5);
 
-        assertThat(requestLimitRule.getPrecision()).isEqualTo(60);
+        assertThat(requestLimitRule.getPrecisionSeconds()).isEqualTo(60);
     }
 
     @Test
@@ -41,9 +40,9 @@ void shouldHaveLimit5() {
 
     @Test
     void shouldHavePrecisionOf10() {
-        RequestLimitRule requestLimitRule = RequestLimitRule.of(Duration.ofSeconds(1), 5).withPrecision(10);
+        RequestLimitRule requestLimitRule = RequestLimitRule.of(Duration.ofSeconds(1), 5).withPrecision(Duration.ofSeconds(10));
 
-        assertThat(requestLimitRule.getPrecision()).isEqualTo(10);
+        assertThat(requestLimitRule.getPrecisionSeconds()).isEqualTo(10);
     }
 
     @Test

ratelimitj-hazelcast/src/main/java/es/moki/ratelimitj/hazelcast/HazelcastSlidingWindowRequestRateLimiter.java

@@ -110,7 +110,7 @@ private boolean eqOrGeLimit(String key, int weight, boolean strictlyGreater) {
         // TODO perform each rule calculation in parallel
         for (RequestLimitRule rule : rules) {
 
-            SavedKey savedKey = new SavedKey(now, rule.getDurationSeconds(), rule.getPrecision());
+            SavedKey savedKey = new SavedKey(now, rule.getDurationSeconds(), rule.getPrecisionSeconds());
             savedKeys.add(savedKey);
 
             Long oldTs = hcKeyMap.get(savedKey.tsKey);

ratelimitj-inmemory/src/main/java/es/moki/ratelimitj/inmemory/request/InMemorySlidingWindowRequestRateLimiter.java

@@ -4,7 +4,6 @@
 import de.jkeylockmanager.manager.KeyLockManagers;
 import es.moki.ratelimitj.core.limiter.request.DefaultRequestLimitRulesSupplier;
 import es.moki.ratelimitj.core.limiter.request.RequestLimitRule;
-import es.moki.ratelimitj.core.limiter.request.RequestLimitRulesSupplier;
 import es.moki.ratelimitj.core.limiter.request.RequestRateLimiter;
 import es.moki.ratelimitj.core.time.SystemTimeSupplier;
 import es.moki.ratelimitj.core.time.TimeSupplier;
@@ -117,7 +116,7 @@ private boolean eqOrGeLimit(String key, int weight, boolean strictlyGreater) {
         // TODO perform each rule calculation in parallel
         for (RequestLimitRule rule : rules) {
 
-            SavedKey savedKey = new SavedKey(now, rule.getDurationSeconds(), rule.getPrecision());
+            SavedKey savedKey = new SavedKey(now, rule.getDurationSeconds(), rule.getPrecisionSeconds());
             savedKeys.add(savedKey);
 
             Long oldTs = keyMap.get(savedKey.tsKey);

ratelimitj-inmemory/src/test/java/es/moki/ratelimitj/inmemory/request/InMemoryRequestRateLimiterInternalTest.java

@@ -21,7 +21,6 @@
 
 class InMemoryRequestRateLimiterInternalTest {
 
-
     private final TimeBanditSupplier timeBandit = new TimeBanditSupplier();
 
     private final ExpiringMap expiryingKeyMap = ExpiringMap.builder().variableExpiration().build();

ratelimitj-redis/src/main/java/es/moki/ratelimitj/redis/request/LimitRuleJsonSerialiser.java

@@ -17,6 +17,6 @@ private JsonArray toJsonArray(RequestLimitRule rule) {
         return Json.array().asArray()
                 .add(rule.getDurationSeconds())
                 .add(rule.getLimit())
-                .add(rule.getPrecision());
+                .add(rule.getPrecisionSeconds());
     }
 }

ratelimitj-redis/src/test/java/es/moki/ratelimitj/redis/request/RequestLimitRuleJsonSerialiserTest.java

@@ -29,8 +29,8 @@ void shouldEncode() {
     @DisplayName("should encode limit rule with precision in JSON array")
     void shouldEncodeWithPrecisions() {
 
-        ImmutableList<RequestLimitRule> rules = ImmutableList.of(RequestLimitRule.of(Duration.ofSeconds(10), 10L).withPrecision(4),
-                RequestLimitRule.of(Duration.ofMinutes(1), 20L).withPrecision(8));
+        ImmutableList<RequestLimitRule> rules = ImmutableList.of(RequestLimitRule.of(Duration.ofSeconds(10), 10L).withPrecision(Duration.ofSeconds(4)),
+                RequestLimitRule.of(Duration.ofMinutes(1), 20L).withPrecision(Duration.ofSeconds(8)));
 
         assertThat(serialiser.encode(rules)).isEqualTo("[[10,10,4],[60,20,8]]");
     }

ratelimitj-redis/src/test/resources/logback-test.xml

@@ -6,7 +6,7 @@
         </encoder>
     </appender>
 
-    <logger name="es.moki" level="debug"
+    <logger name="es.moki" level="info"
             additivity="false">
         <appender-ref ref="STDOUT"/>
     </logger>

ratelimitj-test/src/main/java/es/moki/ratelimitj/test/limiter/request/AbstractSyncRequestRateLimiterTest.java

@@ -1,14 +1,17 @@
 package es.moki.ratelimitj.test.limiter.request;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
 import es.moki.ratelimitj.core.limiter.request.RequestLimitRule;
 import es.moki.ratelimitj.core.limiter.request.RequestRateLimiter;
 import es.moki.ratelimitj.core.time.TimeSupplier;
 import es.moki.ratelimitj.test.time.TimeBanditSupplier;
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import java.time.Duration;
-import java.util.Set;
+import java.util.*;
+import java.util.function.BiFunction;
 import java.util.stream.IntStream;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -115,4 +118,32 @@ void shouldResetLimit() {
         assertThat(requestRateLimiter.overLimitWhenIncremented(key)).isFalse();
     }
 
+    @Test @Disabled
+    void shouldPreventThunderingHerdWithPrecision() {
+
+        RequestLimitRule rule1 = RequestLimitRule.of(Duration.ofSeconds(5), 250).withPrecision(Duration.ofSeconds(1)).matchingKeys("ip:127.9.9.9");
+        RequestRateLimiter requestRateLimiter = getRateLimiter(ImmutableSet.of(rule1), timeBandit);
+        Map<Long, Integer> underPerSecond = new LinkedHashMap<>();
+        Map<Long, Integer> overPerSecond = new HashMap<>();
+
+        IntStream.rangeClosed(1, 50).forEach(loop -> {
+
+            IntStream.rangeClosed(1, 250).forEach(value -> {
+                timeBandit.addUnixTimeMilliSeconds(14L);
+                boolean overLimit = requestRateLimiter.overLimitWhenIncremented("ip:127.9.9.9");
+                if (!overLimit) {
+                    underPerSecond.merge(timeBandit.get(), 1, Integer::sum);
+                } else {
+                    overPerSecond.merge(timeBandit.get(), 1, Integer::sum);
+                }
+            });
+
+        });
+
+        Set<Long> allSeconds = Sets.newTreeSet(Sets.union(underPerSecond.keySet(), overPerSecond.keySet()));
+
+        allSeconds.forEach((k)->System.out.println("Time seconds : " + k + " under count : " + underPerSecond.get(k) + " over count : " + overPerSecond.get(k)));
+    }
+
+
 }