Merge branch '4.2.x'

Author: ryanjbaxter

docs/modules/ROOT/pages/spring-cloud-gateway-server-webflux/gatewayfilter-factories/retry-factory.adoc

@@ -12,6 +12,9 @@ The `Retry` `GatewayFilter` factory supports the following parameters:
 Retries are performed after a backoff interval of `firstBackoff * (factor ^ n)`, where `n` is the iteration.
 If `maxBackoff` is configured, the maximum backoff applied is limited to `maxBackoff`.
 If `basedOnPreviousValue` is true, the backoff is calculated by using `prevBackoff * factor`.
+* `jitter`: The configured random jitter for the retries.
+Generating a backoff between `[backoff - backoff*randomFactor, backoff + backoff*randomFactor]`
+* `timeout`:  The configured timeout for the retries.
 
 The following defaults are configured for `Retry` filter, if enabled:
 
@@ -20,6 +23,8 @@ The following defaults are configured for `Retry` filter, if enabled:
 * `methods`: GET method
 * `exceptions`: `IOException` and `TimeoutException`
 * `backoff`: disabled
+* `jitter`: disabled
+* `timeout`: unlimited
 
 The following listing configures a Retry `GatewayFilter`:
 
@@ -45,6 +50,9 @@ spring:
               maxBackoff: 50ms
               factor: 2
               basedOnPreviousValue: false
+            jitter:
+              randomFactor: 0.5
+            timeout: 100ms
 ----
 
 NOTE: When using the retry filter with a `forward:` prefixed URL, the target endpoint should be written carefully so that, in case of an error, it does not do anything that could result in a response being sent to the client and committed.
@@ -79,10 +87,13 @@ spring:
               maxBackoff: 50ms
               factor: 2
               basedOnPreviousValue: false
+            jitter:
+              randomFactor: 0.5
+            timeout: 100ms
 
       - id: retryshortcut_route
         uri: https://example.org
         filters:
-        - Retry=3,INTERNAL_SERVER_ERROR,GET,10ms,50ms,2,false
+        - Retry=3,INTERNAL_SERVER_ERROR,GET,10ms,50ms,2,false,0.5,100ms
 ----
 

spring-cloud-gateway-integration-tests/pom.xml

@@ -19,7 +19,7 @@
 		<version>4.3.0-SNAPSHOT</version>
 		<relativePath>..</relativePath> <!-- lookup parent from repository -->
 	</parent>
-	
+
 	<modules>
 		<module>grpc</module>
 		<module>http2</module>

spring-cloud-gateway-server-mvc/pom.xml

@@ -136,4 +136,4 @@
 			<scope>test</scope>
 		</dependency>
     </dependencies>
-</project>
+</project>

spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java

@@ -30,6 +30,7 @@
 import reactor.core.publisher.Mono;
 import reactor.netty.Connection;
 import reactor.retry.Backoff;
+import reactor.retry.Jitter;
 import reactor.retry.Repeat;
 import reactor.retry.RepeatContext;
 import reactor.retry.Retry;
@@ -40,6 +41,7 @@
 import org.springframework.cloud.gateway.support.HasRouteId;
 import org.springframework.cloud.gateway.support.ServerWebExchangeUtils;
 import org.springframework.cloud.gateway.support.TimeoutException;
+import org.springframework.core.style.ToStringCreator;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.HttpStatus.Series;
@@ -69,7 +71,7 @@ private static <T> List<T> toList(T... items) {
 	@Override
 	public List<String> shortcutFieldOrder() {
 		return Arrays.asList("retries", "statuses", "methods", "backoff.firstBackoff", "backoff.maxBackoff",
-				"backoff.factor", "backoff.basedOnPreviousValue");
+				"backoff.factor", "backoff.basedOnPreviousValue", "jitter.randomFactor", "timeout");
 	}
 
 	@Override
@@ -123,10 +125,16 @@ public GatewayFilter apply(RetryConfig retryConfig) {
 			if (backoff != null) {
 				statusCodeRepeat = statusCodeRepeat.backoff(getBackoff(backoff));
 			}
+			JitterConfig jitter = retryConfig.getJitter();
+			if (jitter != null) {
+				statusCodeRepeat = statusCodeRepeat.jitter(getJitter(jitter));
+			}
+			Duration timeout = retryConfig.getTimeout();
+			if (timeout != null) {
+				statusCodeRepeat = statusCodeRepeat.timeout(timeout);
+			}
 		}
 
-		// TODO: support timeout, backoff, jitter, etc... in Builder
-
 		Retry<ServerWebExchange> exceptionRetry = null;
 		if (!retryConfig.getExceptions().isEmpty()) {
 			Predicate<RetryContext<ServerWebExchange>> retryContextPredicate = context -> {
@@ -162,6 +170,14 @@ public GatewayFilter apply(RetryConfig retryConfig) {
 			if (backoff != null) {
 				exceptionRetry = exceptionRetry.backoff(getBackoff(backoff));
 			}
+			JitterConfig jitter = retryConfig.getJitter();
+			if (jitter != null) {
+				exceptionRetry = exceptionRetry.jitter(getJitter(jitter));
+			}
+			Duration timeout = retryConfig.getTimeout();
+			if (timeout != null) {
+				exceptionRetry = exceptionRetry.timeout(timeout);
+			}
 		}
 
 		GatewayFilter gatewayFilter = apply(retryConfig.getRouteId(), statusCodeRepeat, exceptionRetry);
@@ -179,6 +195,9 @@ public String toString() {
 					.append("statuses", retryConfig.getStatuses())
 					.append("methods", retryConfig.getMethods())
 					.append("exceptions", retryConfig.getExceptions())
+					.append("backoff", retryConfig.getBackoff())
+					.append("jitter", retryConfig.getJitter())
+					.append("timeout", retryConfig.getTimeout())
 					.toString();
 			}
 		};
@@ -203,6 +222,10 @@ private Backoff getBackoff(BackoffConfig backoff) {
 				backoff.basedOnPreviousValue);
 	}
 
+	private Jitter getJitter(JitterConfig jitter) {
+		return Jitter.random(jitter.randomFactor);
+	}
+
 	public boolean exceedsMaxIterations(ServerWebExchange exchange, RetryConfig retryConfig) {
 		Integer iteration = exchange.getAttribute(RETRY_ITERATION_KEY);
 
@@ -291,6 +314,10 @@ public static class RetryConfig implements HasRouteId {
 
 		private BackoffConfig backoff;
 
+		private JitterConfig jitter;
+
+		private Duration timeout;
+
 		public RetryConfig allMethods() {
 			return setMethods(HttpMethod.values());
 		}
@@ -303,6 +330,35 @@ public void validate() {
 			if (this.backoff != null) {
 				this.backoff.validate();
 			}
+			if (this.jitter != null) {
+				this.jitter.validate();
+			}
+			if (this.timeout != null) {
+				Assert.isTrue(!timeout.isNegative(), "timeout should be >= 0");
+			}
+		}
+
+		public Duration getTimeout() {
+			return timeout;
+		}
+
+		public RetryConfig setTimeout(Duration timeout) {
+			this.timeout = timeout;
+			return this;
+		}
+
+		public JitterConfig getJitter() {
+			return jitter;
+		}
+
+		public RetryConfig setJitter(JitterConfig jitter) {
+			this.jitter = jitter;
+			return this;
+		}
+
+		public RetryConfig setJitter(double randomFactor) {
+			this.jitter = new JitterConfig(randomFactor);
+			return this;
 		}
 
 		public BackoffConfig getBackoff() {
@@ -433,6 +489,47 @@ public void setBasedOnPreviousValue(boolean basedOnPreviousValue) {
 			this.basedOnPreviousValue = basedOnPreviousValue;
 		}
 
+		@Override
+		public String toString() {
+			return new ToStringCreator(this).append("firstBackoff", firstBackoff)
+				.append("maxBackoff", maxBackoff)
+				.append("factor", factor)
+				.append("basedOnPreviousValue", basedOnPreviousValue)
+				.toString();
+		}
+
+	}
+
+	public static class JitterConfig {
+
+		private double randomFactor = 0.5;
+
+		public void validate() {
+			Assert.isTrue(randomFactor >= 0 && randomFactor <= 1,
+					"random factor must be between 0 and 1 (default 0.5)");
+		}
+
+		public JitterConfig() {
+		}
+
+		public JitterConfig(double randomFactor) {
+			this.randomFactor = randomFactor;
+		}
+
+		public double getRandomFactor() {
+			return randomFactor;
+		}
+
+		public void setRandomFactor(double randomFactor) {
+			this.randomFactor = randomFactor;
+		}
+
+		@Override
+		public String toString() {
+			return new ToStringCreator(this).append("randomFactor", randomFactor).toString();
+
+		}
+
 	}
 
 }

spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java

@@ -109,6 +109,30 @@ public void retryWithBackoff() {
 		// @formatter:on
 	}
 
+	@Test
+	public void retryWithBackoffJitterTimeout() {
+		// @formatter:off
+		testClient.get()
+				.uri("/retry?key=retry-with-backoff-jitter-timeout&count=3")
+				.header(HttpHeaders.HOST, "www.retrywithbackoffjittertimeout.org")
+				.exchange()
+				.expectStatus().isOk()
+				.expectHeader().value("X-Retry-Count", CoreMatchers.equalTo("3"));
+		// @formatter:on
+	}
+
+	@Test
+	public void retryWithBackoffTimeout() {
+		// backoff > timeout
+		testClient.get()
+			.uri("/retry?key=retry-with-backoff-timeout&count=3")
+			.header(HttpHeaders.HOST, "www.retrywithbackofftimeout.org")
+			.exchange()
+			.expectStatus()
+			.isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
+		assertThat(TestConfig.map.get("retry-with-backoff-timeout")).isNotNull().hasValue(2);
+	}
+
 	@Test
 	public void retryFilterGetJavaDsl() {
 		testClient.get()
@@ -363,6 +387,21 @@ public RouteLocator hystrixRouteLocator(RouteLocatorBuilder builder) {
 						r -> r.host("**.retrywithbackoff.org").filters(f -> f.prefixPath("/httpbin").retry(config -> {
 							config.setRetries(2).setBackoff(Duration.ofMillis(100), null, 2, true);
 						})).uri(uri))
+				.route("retry_with_backoff_jitter_timeout_test", r -> r.host("**.retrywithbackoffjittertimeout.org")
+					.filters(f -> f.prefixPath("/httpbin").retry(config -> {
+						config.setRetries(3)
+							.setBackoff(Duration.ofMillis(50), Duration.ofMillis(100), 2, true)
+							.setJitter(0.1)
+							.setTimeout(Duration.ofMillis(1000));
+					}))
+					.uri(uri))
+				.route("retry_with_backoff_timeout_test", r -> r.host("**.retrywithbackofftimeout.org")
+					.filters(f -> f.prefixPath("/httpbin").retry(config -> {
+						config.setRetries(3)
+							.setBackoff(Duration.ofMillis(100), null, 2, true)
+							.setTimeout(Duration.ofMillis(200));
+					}))
+					.uri(uri))
 
 				.route("retry_with_loadbalancer",
 						r -> r.host("**.retrywithloadbalancer.org")