diff --git a/docs/src/main/asciidoc/_configprops.adoc b/docs/src/main/asciidoc/_configprops.adoc index c621c052..b046388d 100644 --- a/docs/src/main/asciidoc/_configprops.adoc +++ b/docs/src/main/asciidoc/_configprops.adoc @@ -36,6 +36,10 @@ |spring.cloud.loadbalancer.health-check.path | | |spring.cloud.loadbalancer.hint | | Allows setting the value of hint that is passed on to the LoadBalancer request and can subsequently be used in {@link ReactiveLoadBalancer} implementations. |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 df2bddf1..2ae87041 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -438,10 +438,16 @@ Then, `ReactiveLoadBalancer` is used underneath. 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. + 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: +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. +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. ==== [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 e116d854..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,20 +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 @@ -79,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 @@ -93,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 feb937e3..737c0e91 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 @@ -38,7 +38,7 @@ import org.springframework.retry.support.RetryTemplate; import org.springframework.web.client.RestTemplate; /** - * Auto-configuration for blocking (client-side load balancing). + * Auto-configuration for blocking client-side load balancing. * * @author Spencer Gibb * @author Dave Syer 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/test/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicyTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/InterceptorRetryPolicyTest.java index 2038582c..b7af67ec 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 { - InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(this.request, this.policy, - this.serviceInstanceChooser, this.serviceName); + public void canRetryBeforeExecution() { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(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 { - InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(this.request, this.policy, - this.serviceInstanceChooser, this.serviceName); + public void canRetryNextServer() { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(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 { - InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(this.request, this.policy, - this.serviceInstanceChooser, this.serviceName); + public void cannotRetry() { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(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 { - InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(this.request, this.policy, - this.serviceInstanceChooser, this.serviceName); + public void open() { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(request, policy, + serviceInstanceChooser, serviceName); RetryContext context = interceptorRetryPolicy.open(null); then(context).isInstanceOf(LoadBalancedRetryContext.class); } @Test - public void close() throws Exception { - InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(this.request, this.policy, - this.serviceInstanceChooser, this.serviceName); + public void close() { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(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 { - InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(this.request, this.policy, - this.serviceInstanceChooser, this.serviceName); + public void registerThrowable() { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(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 { - InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(this.request, this.policy, - this.serviceInstanceChooser, this.serviceName); + public void equals() { + InterceptorRetryPolicy interceptorRetryPolicy = new InterceptorRetryPolicy(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 fed2aaa9..76529410 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..ecf99520 --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryFactory.java @@ -0,0 +1,45 @@ +/* + * 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..c03607df --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicy.java @@ -0,0 +1,97 @@ +/* + * 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 7896c76f..1899cae0 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 @@ -21,14 +21,19 @@ 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.ConditionalOnMissingBean; +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.LoadBalancerClient; +import org.springframework.cloud.client.loadbalancer.LoadBalancerRetryProperties; import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; 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.Configuration; +import org.springframework.retry.support.RetryTemplate; import org.springframework.web.client.RestTemplate; /** @@ -53,4 +58,17 @@ public class BlockingLoadBalancerClientAutoConfiguration { return new BlockingLoadBalancerClient(loadBalancerClientFactory, properties); } + @Configuration + @ConditionalOnClass(RetryTemplate.class) + @EnableConfigurationProperties(LoadBalancerRetryProperties.class) + protected static class BlockingLoadBalancerRetryConfig { + + @Bean + @ConditionalOnMissingBean + LoadBalancedRetryFactory loadBalancedRetryFactory(LoadBalancerRetryProperties retryProperties) { + return new BlockingLoadBalancedRetryFactory(retryProperties); + } + + } + } 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..0c4f9670 --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/retry/BlockingLoadBalancedRetryPolicyTests.java @@ -0,0 +1,124 @@ +/* + * 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 7e540fb8..c2d2ea3f 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.web.client.RestTemplate; @@ -31,15 +32,18 @@ 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() .withConfiguration(AutoConfigurations.of(LoadBalancerAutoConfiguration.class, BlockingLoadBalancerClientAutoConfiguration.class)); @Test - public void beansCreatedNormally() { - applicationContextRunner.run(ctxt -> assertThat(ctxt).hasSingleBean(BlockingLoadBalancerClient.class)); + void beansCreatedNormally() { + applicationContextRunner.run(ctxt -> { + assertThat(ctxt).hasSingleBean(BlockingLoadBalancerClient.class); + assertThat(ctxt).hasSingleBean(LoadBalancedRetryFactory.class); + }); } @Test