RetryTemplate does not work properly when calling multiple serviceNames

[huohuo]

Hi guys.

We encountered following issue when using consul and RetryLoadBalancerInterceptor

Service-A calls service-1 and service-2.
Service-1 and service-2 are provided in different servers with different paths.
service-1 : http://host-1:8888/path1/service1
service-2: http://host-2:9999/path2/service2

But we found that some requests access following URLs which caused 404 errors.
http://host-2:9999/path1/service1
http://host-1:8888/path2/service2

We researched the source code and found that in RetryLoadBalancerInterceptor since the retryTemplate is singleton, the retryPolicy inside retryTemplate maybe modified by another thread before the request is executed.

retryTemplate.setRetryPolicy(
  !lbProperties.isEnabled() || retryPolicy == null ? new NeverRetryPolicy()
	 : new InterceptorRetryPolicy(request, retryPolicy, loadBalancer,
		 serviceName));

Please see the code in RetryTemplate at about line 277 (master branch)
while (canRetry(retryPolicy, context) && !context.isExhaustedOnly())

And InterceptorRetryPolicy canRetry method,

 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(serviceInstanceChooser.choose(serviceName));
            return true;
        }

Since the serviceInstance which contains the real service host and port info maybe modified in canRetry method, it may change the request URL to a host of another serviceName (If another thread modified the retryPolicy with other serviceName. In class ServiceRequestWrapper is where the request URL is actually changed by using ServiceInstance).

So we disabled the retryPolicy by config
spring.cloud.loadbalancer.retry.enabled=false
so that in RetryLoadBalancerInterceptor.intercept we will always use NeverRetryPolicy object, and we have not encountered 404 since then. (We encountered 404 every hour before modifying the configuration).

Would you please confirm if my investigation on our issue is correct or not?

Thank you.

[ryanjbaxter]

@huohuo I think you are right. I think the correct solution here is not to use a RetryTemplate bean but instead create a new RetryTemplate for each request. This is what we have done in Spring Cloud Netflix. Let me look at this today.

[huohuo]

@ryanjbaxter Thank you for your quick action. Great work.