Reuse serviceInstance for retries Fixes gh-840 (#841)

This commit is contained in:
apikozh
2023-03-03 12:54:13 +01:00
committed by GitHub
parent 647b0b3d1a
commit 23ca17df2c
2 changed files with 47 additions and 4 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2022 the original author or authors.
* Copyright 2013-2023 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.
@@ -66,6 +66,7 @@ import static org.springframework.cloud.openfeign.loadbalancer.LoadBalancerUtils
*
* @author Olga Maciaszek-Sharma
* @author Wonsik Cheung
* @author Andriy Pikozh
* @since 2.2.6
*/
@SuppressWarnings({ "rawtypes", "unchecked" })
@@ -125,8 +126,8 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client {
// and extract the server and update the request being made
if (context instanceof LoadBalancedRetryContext) {
LoadBalancedRetryContext lbContext = (LoadBalancedRetryContext) context;
ServiceInstance serviceInstance = lbContext.getServiceInstance();
if (serviceInstance == null) {
retrievedServiceInstance = lbContext.getServiceInstance();
if (retrievedServiceInstance == null) {
if (LOG.isDebugEnabled()) {
LOG.debug("Service instance retrieved from LoadBalancedRetryContext: was null. "
+ "Reattempting service instance selection");

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2013-2022 the original author or authors.
* Copyright 2013-2023 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.
@@ -74,6 +74,7 @@ import static org.mockito.Mockito.when;
* "https://github.com/spring-cloud/spring-cloud-commons/blob/main/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/blocking/client/BlockingLoadBalancerClientTests.java">BlockingLoadBalancerClientTests</a>
* @author Olga Maciaszek-Sharma
* @author Wonsik Cheung
* @author Andriy Pikozh
*/
@ExtendWith(MockitoExtension.class)
class RetryableFeignBlockingLoadBalancerClientTests {
@@ -158,10 +159,51 @@ class RetryableFeignBlockingLoadBalancerClientTests {
feignBlockingLoadBalancerClient.execute(request, new Request.Options());
verify(loadBalancerClient, times(2)).choose(eq("test"), any());
verify(loadBalancerClient, times(2)).reconstructURI(serviceInstance, URI.create("http://test/path"));
verify(delegate, times(2)).execute(any(), any());
}
@Test
void shouldReuseServerInstanceOnSameInstanceRetry() throws IOException {
properties.getRetry().setMaxRetriesOnSameServiceInstance(1);
properties.getRetry().setMaxRetriesOnNextServiceInstance(0);
properties.getRetry().getRetryableStatusCodes().add(503);
Request request = testRequest();
Response response = testResponse(503);
when(delegate.execute(any(), any())).thenReturn(response);
when(retryFactory.createRetryPolicy(any(), eq(loadBalancerClient)))
.thenReturn(new BlockingLoadBalancedRetryPolicy(properties));
when(loadBalancerClient.reconstructURI(serviceInstance, URI.create("http://test/path")))
.thenReturn(URI.create("http://testhost:80/path"));
feignBlockingLoadBalancerClient.execute(request, new Request.Options());
verify(loadBalancerClient, times(1)).choose(eq("test"), any());
verify(loadBalancerClient, times(2)).reconstructURI(serviceInstance, URI.create("http://test/path"));
verify(delegate, times(2)).execute(any(), any());
}
@Test
void shouldReuseServerInstanceOnSameInstanceRetryWithBothSameAndNextRetries() throws IOException {
properties.getRetry().setMaxRetriesOnSameServiceInstance(1);
properties.getRetry().setMaxRetriesOnNextServiceInstance(1);
properties.getRetry().getRetryableStatusCodes().add(503);
Request request = testRequest();
Response response = testResponse(503);
when(delegate.execute(any(), any())).thenReturn(response);
when(retryFactory.createRetryPolicy(any(), eq(loadBalancerClient)))
.thenReturn(new BlockingLoadBalancedRetryPolicy(properties));
when(loadBalancerClient.reconstructURI(serviceInstance, URI.create("http://test/path")))
.thenReturn(URI.create("http://testhost:80/path"));
feignBlockingLoadBalancerClient.execute(request, new Request.Options());
verify(loadBalancerClient, times(2)).choose(eq("test"), any());
verify(loadBalancerClient, times(4)).reconstructURI(serviceInstance, URI.create("http://test/path"));
verify(delegate, times(4)).execute(any(), any());
}
@Test
void shouldNotRetryOnDisabled() throws IOException {
properties.getRetry().setEnabled(false);