From 2b9196667c0e04abb9048c1648b0092d81a4c86e Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Thu, 16 Nov 2017 12:44:26 -0500 Subject: [PATCH] Provide support for BackOffPolicys when retrying requests. (#270) --- .../main/asciidoc/spring-cloud-commons.adoc | 20 +++++++ .../LoadBalancedBackOffPolicyFactory.java | 21 +++++++ .../LoadBalancerAutoConfiguration.java | 11 +++- .../RetryLoadBalancerInterceptor.java | 20 +++++++ ...tryLoadBalancerAutoConfigurationTests.java | 11 ++++ .../RetryLoadBalancerInterceptorTest.java | 55 ++++++++++++++++--- 6 files changed, 129 insertions(+), 9 deletions(-) create mode 100644 spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedBackOffPolicyFactory.java diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index 27bd28e6..6653a8e6 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -405,6 +405,26 @@ The properties you can use are `client.ribbon.MaxAutoRetries`, See the https://github.com/Netflix/ribbon/wiki/Getting-Started#the-properties-file-sample-clientproperties[Ribbon documentation] for a description of what there properties do. +If you would like to implement a `BackOffPolicy` in your retries you will need to +create a bean of type `LoadBalancedBackOffPolicyFactory`, and return the `BackOffPolicy` +you would like to use for a given service. + +[source,java,indent=0] +---- +@Configuration +public class MyConfiguration { + @Bean + LoadBalancedBackOffPolicyFactory backOffPolciyFactory() { + return new LoadBalancedBackOffPolicyFactory() { + @Override + public BackOffPolicy createBackOffPolicy(String service) { + return new ExponentialBackOffPolicy(); + } + }; + } +} +---- + NOTE: `client` in the above examples should be replaced with your Ribbon client's name. diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedBackOffPolicyFactory.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedBackOffPolicyFactory.java new file mode 100644 index 00000000..9e83cfab --- /dev/null +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancedBackOffPolicyFactory.java @@ -0,0 +1,21 @@ +package org.springframework.cloud.client.loadbalancer; + +import org.springframework.retry.backoff.BackOffPolicy; +import org.springframework.retry.backoff.NoBackOffPolicy; + +/** + * Factory class to return the backoff policy. + * @author Ryan Baxter + */ +public interface LoadBalancedBackOffPolicyFactory { + + public BackOffPolicy createBackOffPolicy(String service); + + static class NoBackOffPolicyFactory implements LoadBalancedBackOffPolicyFactory { + + @Override + public BackOffPolicy createBackOffPolicy(String service) { + return new NoBackOffPolicy(); + } + } +} 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 9f4cb42d..1285abe7 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 @@ -116,6 +116,12 @@ public class LoadBalancerAutoConfiguration { public LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory() { return new LoadBalancedRetryPolicyFactory.NeverRetryFactory(); } + + @Bean + @ConditionalOnMissingBean + public LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory() { + return new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + } } @Configuration @@ -126,9 +132,10 @@ public class LoadBalancerAutoConfiguration { public RetryLoadBalancerInterceptor ribbonInterceptor( LoadBalancerClient loadBalancerClient, LoadBalancerRetryProperties properties, LoadBalancedRetryPolicyFactory lbRetryPolicyFactory, - LoadBalancerRequestFactory requestFactory) { + LoadBalancerRequestFactory requestFactory, + LoadBalancedBackOffPolicyFactory backOffPolicyFactory) { return new RetryLoadBalancerInterceptor(loadBalancerClient, properties, - lbRetryPolicyFactory, requestFactory); + lbRetryPolicyFactory, requestFactory, backOffPolicyFactory); } @Bean 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 ab584cf2..f33ab29f 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 @@ -26,6 +26,8 @@ import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.http.client.ClientHttpResponse; import org.springframework.retry.RetryCallback; import org.springframework.retry.RetryContext; +import org.springframework.retry.backoff.BackOffPolicy; +import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.support.RetryTemplate; import org.springframework.util.Assert; @@ -41,8 +43,11 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto private LoadBalancerClient loadBalancer; private LoadBalancerRetryProperties lbProperties; private LoadBalancerRequestFactory requestFactory; + private LoadBalancedBackOffPolicyFactory backOffPolicyFactory; + @Deprecated + //TODO remove in 2.0.x public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, LoadBalancerRetryProperties lbProperties, LoadBalancedRetryPolicyFactory lbRetryPolicyFactory, @@ -51,6 +56,19 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto this.lbRetryPolicyFactory = lbRetryPolicyFactory; this.lbProperties = lbProperties; this.requestFactory = requestFactory; + this.backOffPolicyFactory = new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); + } + + public RetryLoadBalancerInterceptor(LoadBalancerClient loadBalancer, + LoadBalancerRetryProperties lbProperties, + LoadBalancedRetryPolicyFactory lbRetryPolicyFactory, + LoadBalancerRequestFactory requestFactory, + LoadBalancedBackOffPolicyFactory backOffPolicyFactory) { + this.loadBalancer = loadBalancer; + this.lbRetryPolicyFactory = lbRetryPolicyFactory; + this.lbProperties = lbProperties; + this.requestFactory = requestFactory; + this.backOffPolicyFactory = backOffPolicyFactory; } @Override @@ -62,6 +80,8 @@ public class RetryLoadBalancerInterceptor implements ClientHttpRequestIntercepto final LoadBalancedRetryPolicy retryPolicy = lbRetryPolicyFactory.create(serviceName, loadBalancer); RetryTemplate template = this.retryTemplate == null ? new RetryTemplate() : this.retryTemplate; + BackOffPolicy backOffPolicy = backOffPolicyFactory.createBackOffPolicy(serviceName); + template.setBackOffPolicy(backOffPolicy == null ? new NoBackOffPolicy() : backOffPolicy); template.setThrowLastExceptionOnExhausted(true); template.setRetryPolicy( !lbProperties.isEnabled() || retryPolicy == null ? new NeverRetryPolicy() diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerAutoConfigurationTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerAutoConfigurationTests.java index 9d51bbb2..a3072ee1 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerAutoConfigurationTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerAutoConfigurationTests.java @@ -2,7 +2,10 @@ package org.springframework.cloud.client.loadbalancer; import java.util.List; +import org.junit.Test; +import org.springframework.context.ConfigurableApplicationContext; import org.springframework.http.client.ClientHttpRequestInterceptor; +import org.springframework.retry.backoff.NoBackOffPolicy; import org.springframework.web.client.RestTemplate; import static org.hamcrest.MatcherAssert.assertThat; @@ -21,5 +24,13 @@ public class RetryLoadBalancerAutoConfigurationTests extends AbstractLoadBalance ClientHttpRequestInterceptor interceptor = interceptors.get(0); assertThat(interceptor, is(instanceOf(RetryLoadBalancerInterceptor.class))); } + + @Test + public void testDefaultBackOffPolicy() throws Exception { + ConfigurableApplicationContext context = init(OneRestTemplate.class); + LoadBalancedBackOffPolicyFactory loadBalancedBackOffPolicyFactory = context.getBean(LoadBalancedBackOffPolicyFactory.class); + assertThat(loadBalancedBackOffPolicyFactory, is(instanceOf(LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory.class))); + assertThat(loadBalancedBackOffPolicyFactory.createBackOffPolicy("foo"), is(instanceOf(NoBackOffPolicy.class))); + } } diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java index 47dc1c1a..e749c225 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/RetryLoadBalancerInterceptorTest.java @@ -14,6 +14,11 @@ import org.springframework.http.HttpStatus; import org.springframework.http.client.ClientHttpRequestExecution; import org.springframework.http.client.ClientHttpResponse; import org.springframework.mock.http.client.MockClientHttpResponse; +import org.springframework.retry.RetryContext; +import org.springframework.retry.backoff.BackOffContext; +import org.springframework.retry.backoff.BackOffInterruptedException; +import org.springframework.retry.backoff.BackOffPolicy; +import org.springframework.retry.backoff.ExponentialBackOffPolicy; import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.support.RetryTemplate; @@ -36,6 +41,7 @@ public class RetryLoadBalancerInterceptorTest { private LoadBalancerClient client; private LoadBalancerRetryProperties lbProperties; private LoadBalancerRequestFactory lbRequestFactory; + private LoadBalancedBackOffPolicyFactory backOffPolicyFactory = new LoadBalancedBackOffPolicyFactory.NoBackOffPolicyFactory(); @Before public void setUp() throws Exception { @@ -62,7 +68,8 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()); lbProperties.setEnabled(false); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, + lbRequestFactory, backOffPolicyFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); interceptor.intercept(request, body, execution); @@ -82,7 +89,8 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo_underscore"))).thenReturn(serviceInstance); when(client.execute(eq("foo_underscore"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory, + backOffPolicyFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); @@ -99,7 +107,8 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, + lbRequestFactory, backOffPolicyFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); interceptor.intercept(request, body, execution); @@ -119,7 +128,8 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, + lbRequestFactory, backOffPolicyFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); @@ -145,7 +155,8 @@ public class RetryLoadBalancerInterceptorTest { when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))). thenReturn(clientHttpResponseNotFound).thenReturn(clientHttpResponseOk); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, + lbRequestFactory, backOffPolicyFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); @@ -164,17 +175,22 @@ public class RetryLoadBalancerInterceptorTest { when(policy.canRetryNextServer(any(LoadBalancedRetryContext.class))).thenReturn(true); LoadBalancedRetryPolicyFactory lbRetryPolicyFactory = mock(LoadBalancedRetryPolicyFactory.class); when(lbRetryPolicyFactory.create(eq("foo"), any(ServiceInstanceChooser.class))).thenReturn(policy); + LoadBalancedBackOffPolicyFactory backOffPolicyFactory = mock(LoadBalancedBackOffPolicyFactory.class); + MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); + when(backOffPolicyFactory.createBackOffPolicy(eq("foo"))).thenReturn(backOffPolicy); ServiceInstance serviceInstance = mock(ServiceInstance.class); when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory, + backOffPolicyFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); verify(client, times(2)).execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class)); assertThat(rsp, is(clientHttpResponse)); verify(lbRequestFactory, times(2)).createRequest(request, body, execution); + assertThat(backOffPolicy.getBackoffAttempts(), is(1)); } @Test(expected = IOException.class) @@ -191,10 +207,35 @@ public class RetryLoadBalancerInterceptorTest { when(client.choose(eq("foo"))).thenReturn(serviceInstance); when(client.execute(eq("foo"), eq(serviceInstance), any(LoadBalancerRequest.class))).thenThrow(new IOException()).thenReturn(clientHttpResponse); lbProperties.setEnabled(true); - RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, lbRequestFactory); + RetryLoadBalancerInterceptor interceptor = new RetryLoadBalancerInterceptor(client, lbProperties, lbRetryPolicyFactory, + lbRequestFactory, backOffPolicyFactory); byte[] body = new byte[]{}; ClientHttpRequestExecution execution = mock(ClientHttpRequestExecution.class); ClientHttpResponse rsp = interceptor.intercept(request, body, execution); verify(lbRequestFactory).createRequest(request, body, execution); } + + class MyBackOffPolicy implements BackOffPolicy { + + private int backoffAttempts = 0; + + @Override + public BackOffContext start(RetryContext retryContext) { + return new BackOffContext() { + @Override + protected Object clone() throws CloneNotSupportedException { + return super.clone(); + } + }; + } + + @Override + public void backOff(BackOffContext backOffContext) throws BackOffInterruptedException { + backoffAttempts++; + } + + public int getBackoffAttempts() { + return backoffAttempts; + } + } } \ No newline at end of file