Add retry support for blocking LoadBalancer

docs/src/main/asciidoc/_configprops.adoc

@@ -35,6 +35,10 @@
 |spring.cloud.loadbalancer.health-check.interval | 25s | Interval for rerunning the HealthCheck scheduler.
 |spring.cloud.loadbalancer.health-check.path |  | 
 |spring.cloud.loadbalancer.retry.enabled | true | 
+|spring.cloud.loadbalancer.retry.max-retries-on-next-service-instance | 1 | Number of retries to be executed on the next <code>ServiceInstance</code>. A <code>ServiceInstance</code> is chosen before each retry call.
+|spring.cloud.loadbalancer.retry.max-retries-on-same-service-instance | 0 | Number of retries to be executed on the same <code>ServiceInstance</code>.
+|spring.cloud.loadbalancer.retry.retry-on-all-operations | false | Indicates retries should be attempted on operations other than {@link HttpMethod#GET}.
+|spring.cloud.loadbalancer.retry.retryable-status-codes |  | A {@link Set} of status codes that should trigger a retry.
 |spring.cloud.loadbalancer.ribbon.enabled | true | Causes `RibbonLoadBalancerClient` to be used by default.
 |spring.cloud.loadbalancer.service-discovery.timeout |  | String representation of Duration of the timeout for calls to service discovery.
 |spring.cloud.loadbalancer.zone |  | Spring Cloud LoadBalancer zone.

docs/src/main/asciidoc/spring-cloud-commons.adoc

@@ -453,12 +453,27 @@ set the `spring.cloud.loadbalancer.ribbon.enabled` property to `false`.
 A load-balanced `RestTemplate` can be configured to retry failed requests.
 By default, this logic is disabled.
 You can enable it by adding link:https://github.com/spring-projects/spring-retry[Spring Retry] to your application's classpath.
-The load-balanced `RestTemplate` honors some of the Ribbon configuration values related to retrying failed requests.
-You can use `client.ribbon.MaxAutoRetries`, `client.ribbon.MaxAutoRetriesNextServer`, and `client.ribbon.OkToRetryOnAllOperations` properties.
+
 If you would like to disable the retry logic with Spring Retry on the classpath, you can set `spring.cloud.loadbalancer.retry.enabled=false`.
+
+If you would like to implement a `BackOffPolicy` in your retries, you need to create a bean of type `LoadBalancedRetryFactory` and override the `createBackOffPolicy()` method.
+
+===== Ribbon-based retries
+
+For the Ribbon-backed implementation, the load-balanced `RestTemplate` honors some of the Ribbon configuration values related to retrying failed requests.
+You can use `client.ribbon.MaxAutoRetries`, `client.ribbon.MaxAutoRetriesNextServer`, and `client.ribbon.OkToRetryOnAllOperations` properties.
+
 See the https://github.com/Netflix/ribbon/wiki/Getting-Started#the-properties-file-sample-clientproperties[Ribbon documentation] for a description of what these properties do.
 
-If you would like to implement a `BackOffPolicy` in your retries, you need to create a bean of type `LoadBalancedRetryFactory` and override the `createBackOffPolicy` method:
+===== Spring Cloud LoadBalancer-based retries
+
+For the Spring Cloud LoadBalancer-backed implementation, you can set:
+
+- `spring.cloud.loadbalancer.retry.maxRetriesOnSameServiceInstance` - indicates how many times a request should be retried on the same ServiceInstance (counted separately for every selected instance)
+- `spring.cloud.loadbalancer.retry.maxRetriesOnNextServiceInstance` - indicates how many times a request should be retried a newly selected ServiceInstance
+- `spring.cloud.loadbalancer.retry.retryableStatusCodes` - the status codes on which to always retry a failed request.
+
+WARN:: If you chose to override the `LoadBalancedRetryFactory` while using the Spring Cloud LoadBalancer-backed approach, make sure you annotate your bean with `@Order` and set it to a higher precedence than `1000`, which is the order set on the `BlockingLoadBalancedRetryFactory`.
 
 ====
 [source,java,indent=0]

spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicy.java

@@ -28,13 +28,13 @@
  */
 public class InterceptorRetryPolicy implements RetryPolicy {
 
-	private HttpRequest request;
+	private final HttpRequest request;
 
-	private LoadBalancedRetryPolicy policy;
+	private final LoadBalancedRetryPolicy policy;
 
-	private ServiceInstanceChooser serviceInstanceChooser;
+	private final ServiceInstanceChooser serviceInstanceChooser;
 
-	private String serviceName;
+	private final String serviceName;
 
 	/**
 	 * Creates a new retry policy.
@@ -56,21 +56,20 @@ public boolean canRetry(RetryContext context) {
 		LoadBalancedRetryContext lbContext = (LoadBalancedRetryContext) context;
 		if (lbContext.getRetryCount() == 0 && lbContext.getServiceInstance() == null) {
 			// We haven't even tried to make the request yet so return true so we do
-			lbContext.setServiceInstance(
-					this.serviceInstanceChooser.choose(this.serviceName));
+			lbContext.setServiceInstance(serviceInstanceChooser.choose(serviceName));
 			return true;
 		}
-		return this.policy.canRetryNextServer(lbContext);
+		return policy.canRetryNextServer(lbContext);
 	}
 
 	@Override
 	public RetryContext open(RetryContext parent) {
-		return new LoadBalancedRetryContext(parent, this.request);
+		return new LoadBalancedRetryContext(parent, request);
 	}
 
 	@Override
 	public void close(RetryContext context) {
-		this.policy.close((LoadBalancedRetryContext) context);
+		policy.close((LoadBalancedRetryContext) context);
 	}
 
 	@Override
@@ -80,7 +79,7 @@ public void registerThrowable(RetryContext context, Throwable throwable) {
 		// increases the retry count
 		lbContext.registerThrowable(throwable);
 		// let the policy know about the exception as well
-		this.policy.registerThrowable(lbContext, throwable);
+		policy.registerThrowable(lbContext, throwable);
 	}
 
 	@Override
@@ -94,25 +93,25 @@ public boolean equals(Object o) {
 
 		InterceptorRetryPolicy that = (InterceptorRetryPolicy) o;
 
-		if (!this.request.equals(that.request)) {
+		if (!request.equals(that.request)) {
 			return false;
 		}
-		if (!this.policy.equals(that.policy)) {
+		if (!policy.equals(that.policy)) {
 			return false;
 		}
-		if (!this.serviceInstanceChooser.equals(that.serviceInstanceChooser)) {
+		if (!serviceInstanceChooser.equals(that.serviceInstanceChooser)) {
 			return false;
 		}
-		return this.serviceName.equals(that.serviceName);
+		return serviceName.equals(that.serviceName);
 
 	}
 
 	@Override
 	public int hashCode() {
-		int result = this.request.hashCode();
-		result = 31 * result + this.policy.hashCode();
-		result = 31 * result + this.serviceInstanceChooser.hashCode();
-		result = 31 * result + this.serviceName.hashCode();
+		int result = request.hashCode();
+		result = 31 * result + policy.hashCode();
+		result = 31 * result + serviceInstanceChooser.hashCode();
+		result = 31 * result + serviceName.hashCode();
 		return result;
 	}
 

spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java

@@ -30,12 +30,13 @@
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.core.annotation.AnnotationAwareOrderComparator;
 import org.springframework.http.client.ClientHttpRequestInterceptor;
 import org.springframework.retry.support.RetryTemplate;
 import org.springframework.web.client.RestTemplate;
 
 /**
- * Auto-configuration for Ribbon (client-side load balancing).
+ * Auto-configuration for blocking client-side load balancing.
  *
  * @author Spencer Gibb
  * @author Dave Syer
@@ -79,7 +80,7 @@ public LoadBalancerRequestFactory loadBalancerRequestFactory(
 	static class LoadBalancerInterceptorConfig {
 
 		@Bean
-		public LoadBalancerInterceptor ribbonInterceptor(
+		public LoadBalancerInterceptor loadBalancerInterceptor(
 				LoadBalancerClient loadBalancerClient,
 				LoadBalancerRequestFactory requestFactory) {
 			return new LoadBalancerInterceptor(loadBalancerClient, requestFactory);
@@ -124,13 +125,14 @@ public static class RetryInterceptorAutoConfiguration {
 
 		@Bean
 		@ConditionalOnMissingBean
-		public RetryLoadBalancerInterceptor ribbonInterceptor(
+		public RetryLoadBalancerInterceptor loadBalancerRetryInterceptor(
 				LoadBalancerClient loadBalancerClient,
 				LoadBalancerRetryProperties properties,
 				LoadBalancerRequestFactory requestFactory,
-				LoadBalancedRetryFactory loadBalancedRetryFactory) {
+				List<LoadBalancedRetryFactory> loadBalancedRetryFactories) {
+			AnnotationAwareOrderComparator.sort(loadBalancedRetryFactories);
 			return new RetryLoadBalancerInterceptor(loadBalancerClient, properties,
-					requestFactory, loadBalancedRetryFactory);
+					requestFactory, loadBalancedRetryFactories.get(0));
 		}
 
 		@Bean

spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRetryProperties.java

@@ -16,7 +16,11 @@
 
 package org.springframework.cloud.client.loadbalancer;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.http.HttpMethod;
 
 /**
  * Configuration properties for the {@link LoadBalancerClient}.
@@ -28,6 +32,28 @@ public class LoadBalancerRetryProperties {
 
 	private boolean enabled = true;
 
+	/**
+	 * Indicates retries should be attempted on operations other than
+	 * {@link HttpMethod#GET}.
+	 */
+	private boolean retryOnAllOperations = false;
+
+	/**
+	 * Number of retries to be executed on the same <code>ServiceInstance</code>.
+	 */
+	private int maxRetriesOnSameServiceInstance = 0;
+
+	/**
+	 * Number of retries to be executed on the next <code>ServiceInstance</code>. A
+	 * <code>ServiceInstance</code> is chosen before each retry call.
+	 */
+	private int maxRetriesOnNextServiceInstance = 1;
+
+	/**
+	 * A {@link Set} of status codes that should trigger a retry.
+	 */
+	private Set<Integer> retryableStatusCodes = new HashSet<>();
+
 	/**
 	 * Returns true if the load balancer should retry failed requests.
 	 * @return True if the load balancer should retry failed requests; false otherwise.
@@ -44,4 +70,36 @@ public void setEnabled(boolean enabled) {
 		this.enabled = enabled;
 	}
 
+	public boolean isRetryOnAllOperations() {
+		return retryOnAllOperations;
+	}
+
+	public void setRetryOnAllOperations(boolean retryOnAllOperations) {
+		this.retryOnAllOperations = retryOnAllOperations;
+	}
+
+	public int getMaxRetriesOnSameServiceInstance() {
+		return maxRetriesOnSameServiceInstance;
+	}
+
+	public void setMaxRetriesOnSameServiceInstance(int maxRetriesOnSameServiceInstance) {
+		this.maxRetriesOnSameServiceInstance = maxRetriesOnSameServiceInstance;
+	}
+
+	public int getMaxRetriesOnNextServiceInstance() {
+		return maxRetriesOnNextServiceInstance;
+	}
+
+	public void setMaxRetriesOnNextServiceInstance(int maxRetriesOnNextServiceInstance) {
+		this.maxRetriesOnNextServiceInstance = maxRetriesOnNextServiceInstance;
+	}
+
+	public Set<Integer> getRetryableStatusCodes() {
+		return retryableStatusCodes;
+	}
+
+	public void setRetryableStatusCodes(Set<Integer> retryableStatusCodes) {
+		this.retryableStatusCodes = retryableStatusCodes;
+	}
+
 }

spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java

@@ -39,13 +39,13 @@
  */
 public class RetryLoadBalancerInterceptor implements ClientHttpRequestInterceptor {
 
-	private LoadBalancerClient loadBalancer;
+	private final LoadBalancerClient loadBalancer;
 
-	private LoadBalancerRetryProperties lbProperties;
+	private final LoadBalancerRetryProperties lbProperties;
 
-	private LoadBalancerRequestFactory requestFactory;
+	private final LoadBalancerRequestFactory requestFactory;
 
-	private LoadBalancedRetryFactory lbRetryFactory;
+	private final LoadBalancedRetryFactory lbRetryFactory;
 
 	public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer,
 			LoadBalancerRetryProperties lbProperties,
@@ -65,8 +65,8 @@ public ClientHttpResponse intercept(final HttpRequest request, final byte[] body
 		final String serviceName = originalUri.getHost();
 		Assert.state(serviceName != null,
 				"Request URI does not contain a valid hostname: " + originalUri);
-		final LoadBalancedRetryPolicy retryPolicy = this.lbRetryFactory
-				.createRetryPolicy(serviceName, this.loadBalancer);
+		final LoadBalancedRetryPolicy retryPolicy = lbRetryFactory
+				.createRetryPolicy(serviceName, loadBalancer);
 		RetryTemplate template = createRetryTemplate(serviceName, request, retryPolicy);
 		return template.execute(context -> {
 			ServiceInstance serviceInstance = null;
@@ -75,11 +75,11 @@ public ClientHttpResponse intercept(final HttpRequest request, final byte[] body
 				serviceInstance = lbContext.getServiceInstance();
 			}
 			if (serviceInstance == null) {
-				serviceInstance = this.loadBalancer.choose(serviceName);
+				serviceInstance = loadBalancer.choose(serviceName);
 			}
-			ClientHttpResponse response = RetryLoadBalancerInterceptor.this.loadBalancer
-					.execute(serviceName, serviceInstance,
-							this.requestFactory.createRequest(request, body, execution));
+			ClientHttpResponse response = loadBalancer.execute(serviceName,
+					serviceInstance,
+					requestFactory.createRequest(request, body, execution));
 			int statusCode = response.getRawStatusCode();
 			if (retryPolicy != null && retryPolicy.retryableStatusCode(statusCode)) {
 				byte[] bodyCopy = StreamUtils.copyToByteArray(response.getBody());
@@ -103,19 +103,17 @@ protected ClientHttpResponse createResponse(ClientHttpResponse response,
 	private RetryTemplate createRetryTemplate(String serviceName, HttpRequest request,
 			LoadBalancedRetryPolicy retryPolicy) {
 		RetryTemplate template = new RetryTemplate();
-		BackOffPolicy backOffPolicy = this.lbRetryFactory
-				.createBackOffPolicy(serviceName);
+		BackOffPolicy backOffPolicy = lbRetryFactory.createBackOffPolicy(serviceName);
 		template.setBackOffPolicy(
 				backOffPolicy == null ? new NoBackOffPolicy() : backOffPolicy);
 		template.setThrowLastExceptionOnExhausted(true);
-		RetryListener[] retryListeners = this.lbRetryFactory
-				.createRetryListeners(serviceName);
+		RetryListener[] retryListeners = lbRetryFactory.createRetryListeners(serviceName);
 		if (retryListeners != null && retryListeners.length != 0) {
 			template.setListeners(retryListeners);
 		}
-		template.setRetryPolicy(!this.lbProperties.isEnabled() || retryPolicy == null
+		template.setRetryPolicy(!lbProperties.isEnabled() || retryPolicy == null
 				? new NeverRetryPolicy() : new InterceptorRetryPolicy(request,
-						retryPolicy, this.loadBalancer, serviceName));
+						retryPolicy, loadBalancer, serviceName));
 		return template;
 	}
 

spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java

@@ -16,7 +16,6 @@
 
 package org.springframework.cloud.client.loadbalancer;
 
-import java.io.IOException;
 import java.net.URI;
 import java.util.Collection;
 import java.util.Map;
@@ -156,7 +155,7 @@ public <T> T execute(String serviceId, LoadBalancerRequest<T> request) {
 
 		@Override
 		public <T> T execute(String serviceId, ServiceInstance serviceInstance,
-				LoadBalancerRequest<T> request) throws IOException {
+				LoadBalancerRequest<T> request) {
 			try {
 				return request.apply(choose(serviceId));
 			}