diff --git a/docs/src/main/asciidoc/_configprops.adoc b/docs/src/main/asciidoc/_configprops.adoc index f4892b15..92d5e393 100644 --- a/docs/src/main/asciidoc/_configprops.adoc +++ b/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 ServiceInstance. A ServiceInstance 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 ServiceInstance. +|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. diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index 7c949991..f9d5dd9f 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/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 the `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] diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicy.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicy.java index e9ea5dad..1ef2f0ed 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicy.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicy.java @@ -28,13 +28,13 @@ import org.springframework.retry.RetryPolicy; */ 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 class InterceptorRetryPolicy implements RetryPolicy { 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 class InterceptorRetryPolicy implements RetryPolicy { // 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 class InterceptorRetryPolicy implements RetryPolicy { 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; } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java index b026ac28..9948f34d 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerAutoConfiguration.java @@ -30,12 +30,13 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClas 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 class LoadBalancerAutoConfiguration { static class LoadBalancerInterceptorConfig { @Bean - public LoadBalancerInterceptor ribbonInterceptor( + public LoadBalancerInterceptor loadBalancerInterceptor( LoadBalancerClient loadBalancerClient, LoadBalancerRequestFactory requestFactory) { return new LoadBalancerInterceptor(loadBalancerClient, requestFactory); @@ -124,13 +125,14 @@ public class LoadBalancerAutoConfiguration { @Bean @ConditionalOnMissingBean - public RetryLoadBalancerInterceptor ribbonInterceptor( + public RetryLoadBalancerInterceptor loadBalancerRetryInterceptor( LoadBalancerClient loadBalancerClient, LoadBalancerRetryProperties properties, LoadBalancerRequestFactory requestFactory, - LoadBalancedRetryFactory loadBalancedRetryFactory) { + List loadBalancedRetryFactories) { + AnnotationAwareOrderComparator.sort(loadBalancedRetryFactories); return new RetryLoadBalancerInterceptor(loadBalancerClient, properties, - requestFactory, loadBalancedRetryFactory); + requestFactory, loadBalancedRetryFactories.get(0)); } @Bean diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRetryProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRetryProperties.java index 22f85823..fbded2dd 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerRetryProperties.java +++ b/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 ServiceInstance. + */ + private int maxRetriesOnSameServiceInstance = 0; + + /** + * Number of retries to be executed on the next ServiceInstance. A + * ServiceInstance is chosen before each retry call. + */ + private int maxRetriesOnNextServiceInstance = 1; + + /** + * A {@link Set} of status codes that should trigger a retry. + */ + private Set 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 class LoadBalancerRetryProperties { 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 getRetryableStatusCodes() { + return retryableStatusCodes; + } + + public void setRetryableStatusCodes(Set retryableStatusCodes) { + this.retryableStatusCodes = retryableStatusCodes; + } + } diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java index 252caad5..ca7a4208 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptor.java @@ -39,13 +39,13 @@ import org.springframework.util.StreamUtils; */ 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 class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto 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 class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto 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 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto 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; } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java index c1b7e330..62388c06 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/AbstractLoadBalancerAutoConfigurationTests.java +++ b/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 abstract class AbstractLoadBalancerAutoConfigurationTests { @Override public T execute(String serviceId, ServiceInstance serviceInstance, - LoadBalancerRequest request) throws IOException { + LoadBalancerRequest request) { try { return request.apply(choose(serviceId)); } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicyTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicyTest.java index 01317834..5ab6d3ff 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicyTest.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicyTest.java @@ -20,14 +20,14 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import org.springframework.cloud.client.ServiceInstance; import org.springframework.http.HttpRequest; import org.springframework.retry.RetryContext; import static org.assertj.core.api.BDDAssertions.then; -import static org.mockito.Matchers.eq; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -49,90 +49,89 @@ public class InterceptorRetryPolicyTest { @Before public void setup() { - this.request = mock(HttpRequest.class); - this.policy = mock(LoadBalancedRetryPolicy.class); - this.serviceInstanceChooser = mock(ServiceInstanceChooser.class); - this.serviceName = "foo"; + request = mock(HttpRequest.class); + policy = mock(LoadBalancedRetryPolicy.class); + serviceInstanceChooser = mock(ServiceInstanceChooser.class); + serviceName = "foo"; } @After public void teardown() { - this.request = null; - this.policy = null; - this.serviceInstanceChooser = null; - this.serviceName = null; + request = null; + policy = null; + serviceInstanceChooser = null; + serviceName = null; } @Test - public void canRetryBeforeExecution() throws Exception { + public void canRetryBeforeExecution() { InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy( - this.request, this.policy, this.serviceInstanceChooser, this.serviceName); + request, policy, serviceInstanceChooser, serviceName); LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); when(context.getRetryCount()).thenReturn(0); ServiceInstance serviceInstance = mock(ServiceInstance.class); - when(this.serviceInstanceChooser.choose(eq(this.serviceName))) - .thenReturn(serviceInstance); + when(serviceInstanceChooser.choose(eq(serviceName))).thenReturn(serviceInstance); then(interceptorRetryPolicy.canRetry(context)).isTrue(); verify(context, times(1)).setServiceInstance(eq(serviceInstance)); } @Test - public void canRetryNextServer() throws Exception { + public void canRetryNextServer() { InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy( - this.request, this.policy, this.serviceInstanceChooser, this.serviceName); + request, policy, serviceInstanceChooser, serviceName); LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); when(context.getRetryCount()).thenReturn(1); - when(this.policy.canRetryNextServer(eq(context))).thenReturn(true); + when(policy.canRetryNextServer(eq(context))).thenReturn(true); then(interceptorRetryPolicy.canRetry(context)).isTrue(); } @Test - public void cannotRetry() throws Exception { + public void cannotRetry() { InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy( - this.request, this.policy, this.serviceInstanceChooser, this.serviceName); + request, policy, serviceInstanceChooser, serviceName); LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); when(context.getRetryCount()).thenReturn(1); then(interceptorRetryPolicy.canRetry(context)).isFalse(); } @Test - public void open() throws Exception { + public void open() { InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy( - this.request, this.policy, this.serviceInstanceChooser, this.serviceName); + request, policy, serviceInstanceChooser, serviceName); RetryContext context = interceptorRetryPolicy.open(null); then(context).isInstanceOf(LoadBalancedRetryContext.class); } @Test - public void close() throws Exception { + public void close() { InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy( - this.request, this.policy, this.serviceInstanceChooser, this.serviceName); + request, policy, serviceInstanceChooser, serviceName); LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); interceptorRetryPolicy.close(context); - verify(this.policy, times(1)).close(eq(context)); + verify(policy, times(1)).close(eq(context)); } @Test - public void registerThrowable() throws Exception { + public void registerThrowable() { InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy( - this.request, this.policy, this.serviceInstanceChooser, this.serviceName); + request, policy, serviceInstanceChooser, serviceName); LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); Throwable thrown = new Exception(); interceptorRetryPolicy.registerThrowable(context, thrown); verify(context, times(1)).registerThrowable(eq(thrown)); - verify(this.policy, times(1)).registerThrowable(eq(context), eq(thrown)); + verify(policy, times(1)).registerThrowable(eq(context), eq(thrown)); } @Test - public void equals() throws Exception { + public void equals() { InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy( - this.request, this.policy, this.serviceInstanceChooser, this.serviceName); + request, policy, serviceInstanceChooser, serviceName); then(interceptorRetryPolicy.equals(null)).isFalse(); then(interceptorRetryPolicy.equals(new Object())).isFalse(); then(interceptorRetryPolicy.equals(interceptorRetryPolicy)).isTrue(); - then(interceptorRetryPolicy.equals(new InterceptorRetryPolicy(this.request, - this.policy, this.serviceInstanceChooser, this.serviceName))).isTrue(); + then(interceptorRetryPolicy.equals(new InterceptorRetryPolicy(request, policy, + serviceInstanceChooser, serviceName))).isTrue(); } } diff --git a/spring-cloud-loadbalancer/pom.xml b/spring-cloud-loadbalancer/pom.xml index 752b83e3..4bae0650 100644 --- a/spring-cloud-loadbalancer/pom.xml +++ b/spring-cloud-loadbalancer/pom.xml @@ -77,6 +77,11 @@ ${evictor.version} true + + org.springframework.retry + spring-retry + true + org.springframework.boot spring-boot-starter-test diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryFactory.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryFactory.java new file mode 100644 index 00000000..56b88e9a --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryFactory.java @@ -0,0 +1,47 @@ +/* + * Copyright 2012-2020 the original author or 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 + * + * https://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. + */ + +package org.springframework.cloud.loadbalancer.blocking.retry; + +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; +import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties; +import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser; +import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; + +/** + * An implementation of {@link LoadBalancedRetryFactory} for + * {@link BlockingLoadBalancerClient}. + * + * @author Olga Maciaszek-Sharma + * @since 2.2.6 + */ +public class BlockingLoadBalancedRetryFactory implements LoadBalancedRetryFactory { + + private final LoadBalancerRetryProperties retryProperties; + + public BlockingLoadBalancedRetryFactory(LoadBalancerRetryProperties retryProperties) { + this.retryProperties = retryProperties; + } + + @Override + public LoadBalancedRetryPolicy createRetryPolicy(String serviceId, + ServiceInstanceChooser serviceInstanceChooser) { + return new BlockingLoadBalancedRetryPolicy(serviceId, serviceInstanceChooser, + retryProperties); + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicy.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicy.java new file mode 100644 index 00000000..9d0e5cf8 --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicy.java @@ -0,0 +1,100 @@ +/* + * Copyright 2012-2020 the original author or 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 + * + * https://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. + */ + +package org.springframework.cloud.loadbalancer.blocking.retry; + +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; +import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties; +import org.springframework.cloud.client.loadbalancer.ServiceInstanceChooser; +import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; +import org.springframework.http.HttpMethod; + +/** + * A {@link LoadBalancedRetryPolicy} implementation for + * {@link BlockingLoadBalancerClient}. Based on RibbonLoadBalancedRetryPolicy + * to achieve feature-parity. + * + * @author Olga Maciaszek-Sharma + * @since 2.2.6 + */ +public class BlockingLoadBalancedRetryPolicy implements LoadBalancedRetryPolicy { + + private final LoadBalancerRetryProperties retryProperties; + + private final ServiceInstanceChooser serviceInstanceChooser; + + private final String serviceId; + + private int sameServerCount = 0; + + private int nextServerCount = 0; + + public BlockingLoadBalancedRetryPolicy(String serviceId, + ServiceInstanceChooser serviceInstanceChooser, + LoadBalancerRetryProperties retryProperties) { + this.serviceId = serviceId; + this.serviceInstanceChooser = serviceInstanceChooser; + this.retryProperties = retryProperties; + } + + public boolean canRetry(LoadBalancedRetryContext context) { + HttpMethod method = context.getRequest().getMethod(); + return HttpMethod.GET.equals(method) || retryProperties.isRetryOnAllOperations(); + } + + @Override + public boolean canRetrySameServer(LoadBalancedRetryContext context) { + return sameServerCount < retryProperties.getMaxRetriesOnSameServiceInstance() + && canRetry(context); + } + + @Override + public boolean canRetryNextServer(LoadBalancedRetryContext context) { + // After the failure, we increment first and then check, hence the equality check + return nextServerCount <= retryProperties.getMaxRetriesOnNextServiceInstance() + && canRetry(context); + } + + @Override + public void close(LoadBalancedRetryContext context) { + + } + + @Override + public void registerThrowable(LoadBalancedRetryContext context, Throwable throwable) { + if (!canRetrySameServer(context) && canRetry(context)) { + // Reset same server since we are moving to a new ServiceInstance + sameServerCount = 0; + nextServerCount++; + if (!canRetryNextServer(context)) { + context.setExhaustedOnly(); + } + else { + context.setServiceInstance(serviceInstanceChooser.choose(serviceId)); + } + } + else { + sameServerCount++; + } + } + + @Override + public boolean retryableStatusCode(int statusCode) { + return retryProperties.getRetryableStatusCodes().contains(statusCode); + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java index 8d54d07e..701381c4 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfiguration.java @@ -26,15 +26,21 @@ import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.loadbalancer.AsyncLoadBalancerAutoConfiguration; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; +import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties; import org.springframework.cloud.client.loadbalancer.reactive.OnNoRibbonDefaultCondition; import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClients; import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; +import org.springframework.cloud.loadbalancer.blocking.retry.BlockingLoadBalancedRetryFactory; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; +import org.springframework.core.annotation.Order; +import org.springframework.retry.support.RetryTemplate; import org.springframework.web.client.RestTemplate; /** @@ -73,6 +79,22 @@ public class BlockingLoadBalancerClientAutoConfiguration { return new BlockingLoadBalancerClient(loadBalancerClientFactory); } + @Configuration + @ConditionalOnClass(RetryTemplate.class) + @EnableConfigurationProperties(LoadBalancerRetryProperties.class) + protected static class BlockingLoadBalancerRetryConfig { + + @Bean + // Allow users to override the factory while avoiding loading + // RibbonLoadBalancedRetryFactory. + @Order(1000) + LoadBalancedRetryFactory loadBalancedRetryFactory( + LoadBalancerRetryProperties retryProperties) { + return new BlockingLoadBalancedRetryFactory(retryProperties); + } + + } + } static class BlockingLoadBalancerClientRibbonWarnLogger { diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicyTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicyTests.java new file mode 100644 index 00000000..f7bccfd9 --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicyTests.java @@ -0,0 +1,127 @@ +/* + * Copyright 2013-2020 the original author or 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 + * + * https://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. + */ + +package org.springframework.cloud.loadbalancer.blocking.retry; + +import java.util.Arrays; +import java.util.HashSet; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties; +import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpRequest; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Tests for {@link BlockingLoadBalancedRetryPolicy}. + * + * @author Olga Maciaszek-Sharma + */ +class BlockingLoadBalancedRetryPolicyTests { + + private final BlockingLoadBalancerClient loadBalancerClient = mock( + BlockingLoadBalancerClient.class); + + private final HttpRequest httpRequest = mock(HttpRequest.class); + + private final LoadBalancedRetryContext context = mock(LoadBalancedRetryContext.class); + + private final LoadBalancerRetryProperties retryProperties = new LoadBalancerRetryProperties(); + + private final UnsupportedOperationException exception = new UnsupportedOperationException(); + + @BeforeEach + void setUp() { + when(httpRequest.getMethod()).thenReturn(HttpMethod.GET); + when(context.getRequest()).thenReturn(httpRequest); + } + + @Test + void shouldExecuteIndicatedNumberOfSameAndNextInstanceRetriesAndCloseRetryContext() { + retryProperties.setMaxRetriesOnSameServiceInstance(1); + BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties); + + assertThat(retryPolicy.canRetrySameServer(context)).isTrue(); + assertThat(retryPolicy.canRetryNextServer(context)).isTrue(); + + retryPolicy.registerThrowable(context, exception); + assertThat(retryPolicy.canRetrySameServer(context)).isFalse(); + assertThat(retryPolicy.canRetryNextServer(context)).isTrue(); + + retryPolicy.registerThrowable(context, exception); + assertThat(retryPolicy.canRetrySameServer(context)).isTrue(); + assertThat(retryPolicy.canRetryNextServer(context)).isTrue(); + + retryPolicy.registerThrowable(context, exception); + assertThat(retryPolicy.canRetrySameServer(context)).isFalse(); + assertThat(retryPolicy.canRetryNextServer(context)).isTrue(); + + retryPolicy.registerThrowable(context, exception); + verify(context).setExhaustedOnly(); + verify(context).setServiceInstance(any()); + assertThat(retryPolicy.canRetrySameServer(context)).isTrue(); + assertThat(retryPolicy.canRetryNextServer(context)).isFalse(); + } + + @Test + void shouldNotRetryWhenMethodNotGet() { + when(httpRequest.getMethod()).thenReturn(HttpMethod.POST); + when(context.getRequest()).thenReturn(httpRequest); + BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties); + + boolean canRetry = retryPolicy.canRetry(context); + + assertThat(canRetry).isFalse(); + } + + @Test + void shouldRetryOnPostWhenEnabled() { + when(httpRequest.getMethod()).thenReturn(HttpMethod.POST); + when(context.getRequest()).thenReturn(httpRequest); + retryProperties.setRetryOnAllOperations(true); + BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties); + + boolean canRetry = retryPolicy.canRetry(context); + + assertThat(canRetry).isTrue(); + } + + @Test + void shouldResolveRetryableStatusCode() { + retryProperties.setRetryableStatusCodes(new HashSet<>(Arrays.asList(404, 502))); + BlockingLoadBalancedRetryPolicy retryPolicy = getRetryPolicy(retryProperties); + + boolean retryableStatusCode = retryPolicy.retryableStatusCode(404); + + assertThat(retryableStatusCode).isTrue(); + } + + private BlockingLoadBalancedRetryPolicy getRetryPolicy( + LoadBalancerRetryProperties retryProperties) { + return new BlockingLoadBalancedRetryPolicy("test", loadBalancerClient, + retryProperties); + } + +} diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfigurationTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfigurationTests.java index 38d2a957..896371ac 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfigurationTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/config/BlockingLoadBalancerClientAutoConfigurationTests.java @@ -16,11 +16,12 @@ package org.springframework.cloud.loadbalancer.config; -import org.junit.Test; +import org.junit.jupiter.api.Test; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; import org.springframework.cloud.loadbalancer.config.BlockingLoadBalancerClientAutoConfiguration.BlockingLoadBalancerClientRibbonWarnLogger; import org.springframework.web.client.RestTemplate; @@ -32,7 +33,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Olga Maciaszek-Sharma * @author Tim Ysewyn */ -public class BlockingLoadBalancerClientAutoConfigurationTests { +class BlockingLoadBalancerClientAutoConfigurationTests { private ApplicationContextRunner applicationContextRunner = new ApplicationContextRunner() .withPropertyValues("spring.cloud.loadbalancer.ribbon.enabled=false") @@ -40,16 +41,17 @@ public class BlockingLoadBalancerClientAutoConfigurationTests { BlockingLoadBalancerClientAutoConfiguration.class)); @Test - public void beansCreatedNormally() { + void beansCreatedNormally() { applicationContextRunner.run(ctxt -> { assertThat(ctxt).hasSingleBean(BlockingLoadBalancerClient.class); + assertThat(ctxt).hasSingleBean(LoadBalancedRetryFactory.class); assertThat(ctxt) .doesNotHaveBean(BlockingLoadBalancerClientRibbonWarnLogger.class); }); } @Test - public void worksWithoutSpringWeb() { + void worksWithoutSpringWeb() { applicationContextRunner .withClassLoader(new FilteredClassLoader(RestTemplate.class)) .run(context -> {