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

Author: apikozh

spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java

@@ -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 @@
  *
  * @author Olga Maciaszek-Sharma
  * @author Wonsik Cheung
+ * @author Andriy Pikozh
  * @since 2.2.6
  */
 @SuppressWarnings({ "rawtypes", "unchecked" })
@@ -125,8 +126,8 @@ public Response execute(Request request, Request.Options options) throws IOExcep
 			// 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");

spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java

@@ -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 @@
  * "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 @@ void shouldRetryOnRepeatableStatusCode() throws IOException {
 
 		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);