Support retries for non-loadbalanced requests

[Ferioney]

Java 11 Spring Boot 2.3.10.RELEASE Spring Cloud Hoxton.SR11 Spring Retry 1.2.5.RELEASE

When I create FeignClient with attribute url, load balancer retry doesn't work. In the previous version (Hoxton.SR10) it worked.

Sample: DemoFeignNotRetryApplicationTests

correctRetry test shows that FeignClient without url was retried. retryDoesNotWork test shows that FeignClient with url wasn't retried.

I think it related to RetryableFeignBlockingLoadBalancerClient does not respect 'url' parameter of @FeignClient. @OlgaMaciaszek Could you please check? Thank you!

[OlgaMaciaszek]

Ther retry support was added for LoadBalancer, hence even all the retry-related properties are prefixed with spring.cloud.loadbalancer.retry. The fact that there were retries when url was passed was sort of an accidental side-effect, they were working cause the requests were being passed via the load-balancing client where they should not have been. We could add retries for non-load-balanced requests as a separate feature, though.

[Aloren]

I would say that this is an issue of naming things correctly. spring-cloud-loadbalancer states in it's name and properties names that it does only load-balancing and thus we are having problems with supporting some of the features for non loadbalanced requests (configured via url in Feign client). spring.cloud.loadbalancer properties semantically do not fit for non loadbalanced requests, so I guess there might be other issues in the future with the same root cause :(

[OlgaMaciaszek]

Retries were historically never added as part of the OpenFeign integration. The retry support was introduced in SC Commons as part of the LoadBalancer integrations and added to OpenFeign as part of the effort to support feature parity between the commons LB implementation and the LB implementation used in OpenFeign and that's also where the property comes from. Retries were never supported or requested for non-load-balanced requests in OF. We can introduce this as a new enhancement, though.

[Aloren]

I think, that I wasn't quite obvious with what I was trying to say. My idea is that whole chain of feign+loadbalancer is quite strange in terms of retries, but that was before as well with feign+ribbon. Initially (openfeign + ribbon integration) retries were available both from Feign side via Retryer interface and also via ribbon properties (MaxAutoRetries, MaxAutoRetriesNextServer, OkToRetryOnAllOperations, retryableStatusCodes). It is understandable why those are required, but definitely not user-friendly and error prone to configure retries in different places and making sure that those are in sync. Unfortunately, introducing loadbalancer did not change that concept of retries, instead made it semantically more difficult to understand.

[OlgaMaciaszek]

For now, we can just add support for non-load-balanced retries (although it is not high-priority, as also Retryer support is present); when we publish next major we can think of improving the API and properties: https://github.com/spring-cloud/spring-cloud-openfeign/issues/535.