Clarify host vs endpoint concurrency limits in README and code

palantir · Oct 15, 2024 · f5af29c · f5af29c
1 parent 8e57cf3
commit f5af29c
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 23 deletions.
diff --git a/README.md b/README.md
@@ -175,12 +175,8 @@ _This API is influenced by gRPC's [Java library](https://github.com/grpc/grpc-ja
 ### Concurrency limits
 
 Every request passes through a pair of [AIMD](https://en.wikipedia.org/wiki/Additive_increase/multiplicative_decrease)
-concurrency limiters.
-There are two types of concurrency limiter: per-host, and per-endpoint. The former is based on failures that
-indicate the target host is in a degraded state, and the latter is based on failures that are coupled to
-an individual endpoint.
-Each concurrency limiter operates in conjunction with a queue to stage pending requests until a
-permit becomes available.
+concurrency limiters. There are two types of concurrency limiter: per-host, and per-endpoint. Each concurrency limiter
+operates in conjunction with a queue to stage pending requests until a permit becomes available.
 
 #### Limiter Diagram
 
@@ -207,13 +203,17 @@ permit becomes available.
 
 Each host has a concurrency limiter which protects servers by stopping requests getting out the door on the client-side.
 Permits are decreased after receiving 308 or 501-599 response, or encountering a network error (`IOException`).
-Otherwise, they are increased.
+429 or 500 responses have no change. Otherwise, permits are increased.
+
+Host limits are based on failures that indicate the target host overall is in a degraded state.
 
 #### Endpoint limits
 
-Each endpoint has a concurrency limiter which is distinct for each host. This allows servers to provide backpressure with
-additional specificity in the form of 429 status QoS responses. Permits are decreased after
-receiving a 429 or 500 response code. Otherwise, they are increased.
+Each endpoint has a concurrency limiter which is distinct for each host. This allows servers to provide per-endpoint
+backpressure in the form of 429 status QoS responses. Permits are decreased after receiving a 429 or 500 response code.
+501-599 responses have no change. Otherwise, permits are increased.
+
+Endpoint limits are based on failures that are coupled to an individual endpoint.
 
 ### Node selection strategies
 When configured with multiple uris, Dialogue has several strategies for choosing which upstream to route requests to.

diff --git a/...java/com/palantir/dialogue/core/CautiousIncreaseAggressiveDecreaseConcurrencyLimiter.java b/...java/com/palantir/dialogue/core/CautiousIncreaseAggressiveDecreaseConcurrencyLimiter.java
@@ -99,10 +99,12 @@ enum Behavior {
         HOST_LEVEL() {
             @Override
             void onSuccess(Response result, PermitControl control) {
-                if (Responses.isInternalServerError(result) || Responses.isTooManyRequests(result)) {
+                if (Responses.isTooManyRequests(result) || Responses.isInternalServerError(result)) {
+                    // 429 or 500
                     control.ignore();
                 } else if ((Responses.isQosStatus(result) && !Responses.isTooManyRequests(result))
                         || Responses.isServerErrorRange(result)) {
+                    // 308 with Location header, or 501-599
                     control.dropped();
                 } else {
                     control.success();
@@ -122,8 +124,10 @@ void onFailure(Throwable throwable, PermitControl control) {
             @Override
             void onSuccess(Response result, PermitControl control) {
                 if (Responses.isTooManyRequests(result) || Responses.isInternalServerError(result)) {
+                    // 429 or 500
                     control.dropped();
                 } else if (Responses.isServerErrorRange(result)) {
+                    // 501-599
                     control.ignore();
                 } else {
                     control.success();
@@ -154,10 +158,22 @@ void onFailure(Throwable _throwable, PermitControl control) {
 
     interface PermitControl {
 
+        /**
+         * Indicates that the effect of the request corresponding to this permit on concurrency limits should be
+         * ignored.
+         */
         void ignore();
 
+        /**
+         * Indicates that the request corresponding to this permit was dropped and that the concurrency limit should be
+         * multiplicatively decreased.
+         */
         void dropped();
 
+        /**
+         * Indicates that the request corresponding to this permit was successful and that the concurrency limit should
+         * be additively increased.
+         */
         void success();
     }
 
@@ -187,19 +203,11 @@ public void onFailure(Throwable throwable) {
             behavior.onFailure(throwable, this);
         }
 
-        /**
-         * Indicates that the effect of the request corresponding to this permit on concurrency limits should be
-         * ignored.
-         */
         @Override
         public void ignore() {
             inFlight.decrementAndGet();
         }
 
-        /**
-         * Indicates that the request corresponding to this permit was dropped and that the concurrency limit should be
-         * multiplicatively decreased.
-         */
         @Override
         public void dropped() {
             inFlight.decrementAndGet();
@@ -209,10 +217,6 @@ public void dropped() {
             }
         }
 
-        /**
-         * Indicates that the request corresponding to this permit was successful and that the concurrency limit should
-         * be additively increased.
-         */
         @Override
         public void success() {
             inFlight.decrementAndGet();