Traffic resiliency outstanding improvements (comments) (#2917)

* Traffic resiliency outstanding improvements (comments)
apple · May 11, 2024 · fd65334 · fd65334
1 parent c85a8f4
commit fd65334
Show file tree

Hide file tree

Showing 30 changed files with 820 additions and 680 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -63,6 +63,9 @@ protoGoogleCommonProtosVersion=2.29.0
 javaPoetVersion=1.13.0
 shadowPluginVersion=8.1.1
 
+# resilience4j - jdk8 compat
+resilience4jVersion=1.7.1
+
 # Test dependencies
 jmhCoreVersion=1.37
 jmhPluginVersion=0.7.2
@@ -82,4 +85,3 @@ commonsLangVersion=2.6
 grpcVersion=1.61.1
 javaxAnnotationsApiVersion=1.3.5
 jsonUnitVersion=2.38.0
-resilience4jVersion=1.7.1
diff --git a/servicetalk-capacity-limiter-api/gradle/checkstyle/suppressions.xml b/servicetalk-capacity-limiter-api/gradle/checkstyle/suppressions.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Copyright © 2024 Apple Inc. and the ServiceTalk project authors
+  ~
+  ~ Licensed under the Apache License, Version 2.0 (the "License");
+  ~ you may not use this file except in compliance with the License.
+  ~ You may obtain a copy of the License at
+  ~
+  ~   http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+<!DOCTYPE suppressions PUBLIC
+        "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
+        "https://checkstyle.org/dtds/suppressions_1_2.dtd">
+
+<suppressions>
+    <suppress checks="LineLength"
+              files="io[\\/]servicetalk[\\/]capacity[\\/]limiter[\\/]api[\\/]GradientCapacityLimiter.java"/>
+</suppressions>
diff --git a/...ty-limiter-api/src/main/java/io/servicetalk/capacity/limiter/api/AimdCapacityLimiter.java b/...ty-limiter-api/src/main/java/io/servicetalk/capacity/limiter/api/AimdCapacityLimiter.java
@@ -46,7 +46,8 @@
 /**
  * A client side dynamic {@link CapacityLimiter} that adapts its limit based on a configurable range of concurrency
  * {@link #min} and {@link #max}, and re-evaluates this limit upon a request-drop event
- * (e.g., timeout or rejection due to capacity).
+ * (e.g., timeout or rejection due to capacity). It's not ideal for server-side solutions, due to the slow recover
+ * mechanism it offers, which can lead in significant traffic loss during the recovery window.
  * <p>
  * The limit translates to a concurrency figure, e.g., how many requests can be in-flight simultaneously and doesn't
  * represent a constant rate (i.e., has no notion of time).
@@ -112,7 +113,7 @@ public Ticket tryAcquire(final Classification classification, @Nullable final Co
                 if (pending >= limit || pending == max) {   // prevent pending going above max if limit is fractional
                     ticket = null;
                 } else {
-                    ticket = new DefaultTicket(this, (int) limit - pending);
+                    ticket = new DefaultTicket(this, (int) limit - pending, pending);
                     pending++;
                 }
                 l = limit;
@@ -208,10 +209,12 @@ private static final class DefaultTicket implements Ticket, LimiterState {
         private static final int UNSUPPORTED = -1;
         private final AimdCapacityLimiter provider;
         private final int remaining;
+        private final int pending;
 
-        DefaultTicket(final AimdCapacityLimiter provider, final int remaining) {
+        DefaultTicket(final AimdCapacityLimiter provider, final int remaining, final int pending) {
             this.provider = provider;
             this.remaining = remaining;
+            this.pending = pending;
         }
 
         @Override
@@ -224,6 +227,11 @@ public int remaining() {
             return remaining;
         }
 
+        @Override
+        public int pending() {
+            return pending;
+        }
+
         @Override
         public int completed() {
             provider.onSuccess();

diff --git a/...ter-api/src/main/java/io/servicetalk/capacity/limiter/api/AimdCapacityLimiterBuilder.java b/...ter-api/src/main/java/io/servicetalk/capacity/limiter/api/AimdCapacityLimiterBuilder.java
@@ -20,10 +20,10 @@
 import java.util.function.LongSupplier;
 import javax.annotation.Nullable;
 
+import static io.servicetalk.capacity.limiter.api.Preconditions.ensureBetweenZeroAndOneExclusive;
+import static io.servicetalk.capacity.limiter.api.Preconditions.ensurePositive;
+import static io.servicetalk.capacity.limiter.api.Preconditions.ensureRange;
 import static io.servicetalk.utils.internal.DurationUtils.ensureNonNegative;
-import static io.servicetalk.utils.internal.NumberUtils.ensureBetweenZeroAndOneExclusive;
-import static io.servicetalk.utils.internal.NumberUtils.ensurePositive;
-import static io.servicetalk.utils.internal.NumberUtils.ensureRange;
 import static java.lang.Integer.MAX_VALUE;
 import static java.util.Objects.requireNonNull;
 

diff --git a/...imiter-api/src/main/java/io/servicetalk/capacity/limiter/api/AllowAllCapacityLimiter.java b/...imiter-api/src/main/java/io/servicetalk/capacity/limiter/api/AllowAllCapacityLimiter.java
@@ -19,13 +19,36 @@
 
 import javax.annotation.Nullable;
 
+import static java.lang.Integer.MAX_VALUE;
+
 final class AllowAllCapacityLimiter implements CapacityLimiter {
     static final CapacityLimiter INSTANCE = new AllowAllCapacityLimiter();
     private static final int UNSUPPORTED = -1;
-    private final Ticket noOpToken = new Ticket() {
+    private static final Ticket noOpToken = new DefaultTicket();
+
+    private AllowAllCapacityLimiter() {
+        // Singleton
+    }
+
+    @Override
+    public String name() {
+        return AllowAllCapacityLimiter.class.getSimpleName();
+    }
+
+    @Override
+    public Ticket tryAcquire(final Classification classification, @Nullable final ContextMap context) {
+        return noOpToken;
+    }
+
+    @Override
+    public String toString() {
+        return name();
+    }
+
+    private static final class DefaultTicket implements Ticket, LimiterState {
         @Override
         public LimiterState state() {
-            return null;
+            return this;
         }
 
         @Override
@@ -47,24 +70,15 @@ public int failed(@Nullable final Throwable error) {
         public int ignored() {
             return UNSUPPORTED;
         }
-    };
 
-    private AllowAllCapacityLimiter() {
-        // Singleton
-    }
-
-    @Override
-    public String name() {
-        return AllowAllCapacityLimiter.class.getSimpleName();
-    }
-
-    @Override
-    public Ticket tryAcquire(final Classification classification, @Nullable final ContextMap context) {
-        return noOpToken;
-    }
+        @Override
+        public int pending() {
+            return -1;
+        }
 
-    @Override
-    public String toString() {
-        return name();
+        @Override
+        public int remaining() {
+            return MAX_VALUE;
+        }
     }
 }
diff --git a/...pacity-limiter-api/src/main/java/io/servicetalk/capacity/limiter/api/CapacityLimiter.java b/...pacity-limiter-api/src/main/java/io/servicetalk/capacity/limiter/api/CapacityLimiter.java
@@ -83,6 +83,13 @@ interface LimiterState {
          * @return the remaining allowance of the {@link CapacityLimiter} when the {@link Ticket} was issued.
          */
         int remaining();
+
+        /**
+         * Returns the current pending (in use capacity) demand.
+         * If the pending is unknown a negative value i.e., -1 is allowed to express this.
+         * @return the current pending (in use capacity) demand.
+         */
+        int pending();
     }
 
     /**
@@ -101,7 +108,6 @@ interface Ticket {
          * Representation of the state of the {@link CapacityLimiter} when this {@link Ticket} was issued.
          * @return the {@link LimiterState state} of the limiter at the time this {@link Ticket} was issued.
          */
-        @Nullable
         LimiterState state();
 
         /**

diff --git a/...acity-limiter-api/src/main/java/io/servicetalk/capacity/limiter/api/CapacityLimiters.java b/...acity-limiter-api/src/main/java/io/servicetalk/capacity/limiter/api/CapacityLimiters.java
@@ -57,16 +57,19 @@ public static CapacityLimiter composite(final List<CapacityLimiter> providers) {
      * target limit for this request will be 70% of the 10 = 7. If current consumption is less than 7, the request
      * will be permitted.
      *
+     * @param capacity The fixed capacity value for this limiter.
      * @return A {@link CapacityLimiter} builder to configure the available parameters.
      */
-    public static FixedCapacityLimiterBuilder fixedCapacity() {
-        return new FixedCapacityLimiterBuilder();
+    public static FixedCapacityLimiterBuilder fixedCapacity(final int capacity) {
+        return new FixedCapacityLimiterBuilder(capacity);
     }
 
     /**
      * AIMD is a request drop based dynamic {@link CapacityLimiter} for clients,
      * that adapts its limit based on a configurable range of concurrency and re-evaluates this limit upon
      * a {@link Ticket#dropped() request-drop event (eg. timeout or rejection due to capacity)}.
+     * It's not ideal for server-side solutions, due to the slow recover mechanism it offers, which can lead in
+     * significant traffic loss during the recovery window.
      * <p>
      * The limit translates to a concurrency figure, e.g. how many requests can be in-flight simultaneously and doesn't
      * represent a constant rate (i.e. has no notion of time). Requests per second when that limit is met will be

diff --git a/...y-limiter-api/src/main/java/io/servicetalk/capacity/limiter/api/FixedCapacityLimiter.java b/...y-limiter-api/src/main/java/io/servicetalk/capacity/limiter/api/FixedCapacityLimiter.java
@@ -74,7 +74,7 @@ public Ticket tryAcquire(final Classification classification, @Nullable final Co
                 final int newPending = currPending + 1;
                 if (pendingUpdater.compareAndSet(this, currPending, newPending)) {
                     notifyObserver(newPending);
-                    return new DefaultTicket(this, effectiveLimit - newPending);
+                    return new DefaultTicket(this, effectiveLimit - newPending, newPending);
                 }
             }
         }
@@ -99,10 +99,12 @@ private static final class DefaultTicket implements Ticket, LimiterState {
 
         private final FixedCapacityLimiter fixedCapacityProvider;
         private final int remaining;
+        private final int pending;
 
-        DefaultTicket(final FixedCapacityLimiter fixedCapacityProvider, int remaining) {
+        DefaultTicket(final FixedCapacityLimiter fixedCapacityProvider, final int remaining, final int pending) {
             this.fixedCapacityProvider = fixedCapacityProvider;
             this.remaining = remaining;
+            this.pending = pending;
         }
 
         @Override
@@ -115,6 +117,11 @@ public int remaining() {
             return remaining;
         }
 
+        @Override
+        public int pending() {
+            return pending;
+        }
+
         private int release() {
             final int pending = pendingUpdater.decrementAndGet(fixedCapacityProvider);
             fixedCapacityProvider.notifyObserver(pending);

diff --git a/...er-api/src/main/java/io/servicetalk/capacity/limiter/api/FixedCapacityLimiterBuilder.java b/...er-api/src/main/java/io/servicetalk/capacity/limiter/api/FixedCapacityLimiterBuilder.java
@@ -18,6 +18,7 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import javax.annotation.Nullable;
 
+import static io.servicetalk.utils.internal.NumberUtils.ensureNonNegative;
 import static java.util.Objects.requireNonNull;
 
 /**
@@ -32,6 +33,10 @@ public final class FixedCapacityLimiterBuilder {
     @Nullable
     private StateObserver observer;
 
+    FixedCapacityLimiterBuilder(final int capacity) {
+        this.capacity = ensureNonNegative(capacity, "capacity");
+    }
+
     /**
      * Defines a name for this {@link CapacityLimiter}.
      * @param name the name to be used when building this {@link CapacityLimiter}.
@@ -51,7 +56,7 @@ public FixedCapacityLimiterBuilder name(final String name) {
      * @return {@code this}.
      */
     public FixedCapacityLimiterBuilder capacity(final int capacity) {
-        this.capacity = capacity;
+        this.capacity = ensureNonNegative(capacity, "capacity");
         return this;
     }
 

diff --git a/...imiter-api/src/main/java/io/servicetalk/capacity/limiter/api/GradientCapacityLimiter.java b/...imiter-api/src/main/java/io/servicetalk/capacity/limiter/api/GradientCapacityLimiter.java
@@ -59,8 +59,7 @@
  * <p>
  * The algorithm is heavily influenced by the following prior-art
  * <ul>
- *     <li><a href="https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters
- *     /adaptive_concurrency_filter">Envoy Adaptive Concurrency</a></li>
+ *     <li><a href="https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/adaptive_concurrency_filter">Envoy Adaptive Concurrency</a></li>
  *     <li><a href="https://github.com/Netflix/concurrency-limits">Netflix Concurrency Limits</a></li>
  * </ul>
  */
@@ -133,7 +132,7 @@ public Ticket tryAcquire(final Classification classification, @Nullable final Co
             newLimit = (int) limit;
             if (pending < limit) {
                 newPending = ++pending;
-                ticket = new DefaultTicket(this, newLimit - newPending);
+                ticket = new DefaultTicket(this, newLimit - newPending, newPending);
             }
         }
 
@@ -241,11 +240,13 @@ private static final class DefaultTicket implements Ticket, LimiterState {
         private final long startTime;
         private final GradientCapacityLimiter provider;
         private final int remaining;
+        private final int pending;
 
-        DefaultTicket(final GradientCapacityLimiter provider, final int remaining) {
+        DefaultTicket(final GradientCapacityLimiter provider, final int remaining, final int pending) {
             this.provider = provider;
             this.startTime = provider.timeSource.getAsLong();
             this.remaining = remaining;
+            this.pending = pending;
         }
 
         @Override
@@ -258,6 +259,11 @@ public int remaining() {
             return remaining;
         }
 
+        @Override
+        public int pending() {
+            return pending;
+        }
+
         @Override
         public int completed() {
             return provider.onSuccess(provider.timeSource.getAsLong() - startTime);

diff --git a/...api/src/main/java/io/servicetalk/capacity/limiter/api/GradientCapacityLimiterBuilder.java b/...api/src/main/java/io/servicetalk/capacity/limiter/api/GradientCapacityLimiterBuilder.java
@@ -43,9 +43,9 @@
 import static io.servicetalk.capacity.limiter.api.GradientCapacityLimiterProfiles.DEFAULT_SUSPEND_INCR;
 import static io.servicetalk.capacity.limiter.api.GradientCapacityLimiterProfiles.GREEDY_HEADROOM;
 import static io.servicetalk.capacity.limiter.api.GradientCapacityLimiterProfiles.MIN_SAMPLING_DURATION;
-import static io.servicetalk.utils.internal.NumberUtils.ensureBetweenZeroAndOne;
-import static io.servicetalk.utils.internal.NumberUtils.ensureBetweenZeroAndOneExclusive;
-import static io.servicetalk.utils.internal.NumberUtils.ensureGreaterThan;
+import static io.servicetalk.capacity.limiter.api.Preconditions.ensureBetweenZeroAndOne;
+import static io.servicetalk.capacity.limiter.api.Preconditions.ensureBetweenZeroAndOneExclusive;
+import static io.servicetalk.capacity.limiter.api.Preconditions.ensureGreaterThan;
 import static java.util.Objects.requireNonNull;
 
 /**