Minor improvements for uniform retry-filter (#2004)

* Minor improvements for uniform retry-filter
apple · Dec 9, 2021 · bbae7d0 · bbae7d0
1 parent 7f0b3b3
commit bbae7d0
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 10 deletions.
diff --git a/...etalk-http-netty/src/main/java/io/servicetalk/http/netty/RetryingHttpRequesterFilter.java b/...etalk-http-netty/src/main/java/io/servicetalk/http/netty/RetryingHttpRequesterFilter.java
@@ -57,7 +57,7 @@
 import static io.servicetalk.concurrent.api.SourceAdapters.toSource;
 import static io.servicetalk.http.api.HeaderUtils.DEFAULT_HEADER_FILTER;
 import static io.servicetalk.http.netty.RetryingHttpRequesterFilter.BackOffPolicy.NO_RETRIES;
-import static io.servicetalk.http.netty.RetryingHttpRequesterFilter.BackOffPolicy.ofInstant;
+import static java.lang.Integer.MAX_VALUE;
 import static java.time.Duration.ZERO;
 import static java.time.Duration.ofDays;
 import static java.util.Objects.requireNonNull;
@@ -81,6 +81,10 @@
 public final class RetryingHttpRequesterFilter
         implements StreamingHttpClientFilterFactory, ExecutionStrategyInfluencer<HttpExecutionStrategy> {
 
+    public static final RetryingHttpRequesterFilter DISABLE_RETRIES =
+            new RetryingHttpRequesterFilter(false, true, 0, null,
+                (__, ___) -> NO_RETRIES);
+
     private final boolean waitForLb;
     private final boolean ignoreSdErrors;
     private final int maxTotalRetries;
@@ -259,8 +263,7 @@ public synchronized Throwable fillInStackTrace() {
         @Override
         public String toString() {
             return super.toString() +
-                ", metaData=" + metaData.toString(DEFAULT_HEADER_FILTER) +
-                ", message='" + message + '\'';
+                ", metaData=" + metaData.toString(DEFAULT_HEADER_FILTER);
         }
     }
 
@@ -300,10 +303,20 @@ public static final class BackOffPolicy {
          * Creates a new {@link BackOffPolicy} that retries failures instantly up-to 3 max retries.
          * @return a new {@link BackOffPolicy} that retries failures instantly up-to 3 max retries.
          */
-        public static BackOffPolicy ofInstant() {
+        public static BackOffPolicy ofImmediate() {
             return new BackOffPolicy(null, ZERO, null, null, false, 3);
         }
 
+        /**
+         * Creates a new {@link BackOffPolicy} that retries failures instantly up-to provided max retries.
+         *
+         * @param maxRetries the number of retry attempts for this {@link BackOffPolicy}.
+         * @return a new {@link BackOffPolicy} that retries failures instantly up-to provided max retries.
+         */
+        public static BackOffPolicy ofImmediate(final int maxRetries) {
+            return new BackOffPolicy(null, ZERO, null, null, false, maxRetries);
+        }
+
         /**
          * Special {@link BackOffPolicy} that signals that no retries will be attempted.
          * @return a special {@link BackOffPolicy} that signals that no retries will be attempted.
@@ -520,13 +533,13 @@ public static final class Builder {
         private boolean waitForLb = true;
         private boolean ignoreSdErrors;
 
-        private int maxRetries = 3;
+        private int maxRetries = MAX_VALUE;
 
         @Nullable
         private Function<HttpResponseMetaData, HttpResponseException> responseMapper;
 
         private BiFunction<HttpRequestMetaData, RetryableException, BackOffPolicy>
-                retryRetryableExceptions = (requestMetaData, e) -> ofInstant();
+                retryRetryableExceptions = (requestMetaData, e) -> BackOffPolicy.ofImmediate();
 
         @Nullable
         private BiFunction<HttpRequestMetaData, IOException, BackOffPolicy>
@@ -572,7 +585,10 @@ public Builder ignoreServiceDiscovererErrors(final boolean ignoreSdErrors) {
         /**
          * Set the maximum number of allowed retry operations before giving up, applied as total max across all retry
          * functions (see. {@link #retryDelayedRetries(BiFunction)}, {@link #retryIdempotentRequests(BiFunction)},
-         * {@link #retryRetryableExceptions(BiFunction)}, {@link #retryOther(BiFunction)}).
+         * {@link #retryRetryableExceptions(BiFunction)}, {@link #retryResponses(BiFunction)},
+         * {@link #retryOther(BiFunction)}).
+         *
+         * If not set, max total retries will be infinite.
          *
          * @param maxRetries Maximum number of allowed retries before giving up
          * @return {@code this}

diff --git a/...k-http-netty/src/test/java/io/servicetalk/http/netty/RetryingHttpRequesterFilterTest.java b/...k-http-netty/src/test/java/io/servicetalk/http/netty/RetryingHttpRequesterFilterTest.java
@@ -54,7 +54,7 @@
 import static io.servicetalk.http.netty.HttpClients.forSingleAddress;
 import static io.servicetalk.http.netty.HttpServers.forAddress;
 import static io.servicetalk.http.netty.RetryingHttpRequesterFilter.BackOffPolicy.NO_RETRIES;
-import static io.servicetalk.http.netty.RetryingHttpRequesterFilter.BackOffPolicy.ofInstant;
+import static io.servicetalk.http.netty.RetryingHttpRequesterFilter.BackOffPolicy.ofImmediate;
 import static io.servicetalk.http.netty.RetryingHttpRequesterFilter.Builder;
 import static io.servicetalk.http.netty.RetryingHttpRequesterFilter.HttpResponseException;
 import static io.servicetalk.transport.netty.internal.AddressUtils.localAddress;
@@ -128,7 +128,7 @@ void requestRetryingPredicate() {
                 .appendClientFilter(new Builder()
                         .retryRetryableExceptions((requestMetaData, e) -> NO_RETRIES)
                         .retryOther((requestMetaData, throwable) ->
-                                requestMetaData.requestTarget().equals("/retry") ? ofInstant() : NO_RETRIES).build())
+                                requestMetaData.requestTarget().equals("/retry") ? ofImmediate() : NO_RETRIES).build())
                 .buildBlocking();
         try {
             failingClient.request(failingClient.get("/"));
@@ -158,7 +158,7 @@ void responseRetryingPredicate() {
                         // Disable request retrying
                         .retryRetryableExceptions((requestMetaData, e) -> NO_RETRIES)
                         // Retry only responses marked so
-                        .retryResponses((requestMetaData, throwable) -> ofInstant())
+                        .retryResponses((requestMetaData, throwable) -> ofImmediate())
                         .build())
                 .buildBlocking();
         try {