Make DelegatingServiceInstanceListSupplier implement SelectedInstanceCallback

[HJK181]

I've tried to setup a custom load balancer as described in this post. However, with spring-cloud-starter-loadbalancer:3.1.4 I'm experiencing

org.springframework.beans.factory.NoUniqueBeanDefinitionException: No qualifying bean of type 'org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier' available: more than one 'primary' bean found among candidates: [CustomServiceInstanceListSupplier, retryAwareDiscoveryClientServiceInstanceListSupplier]

@Configuration
@LoadBalancerClient(value = "custom", configuration = CustomLoadBalancerConfiguration.class)
public class CustomLoadBalancerClient {
}

public class CustomLoadBalancerConfiguration {

	@Bean
	@Primary
	public ServiceInstanceListSupplier customServiceInstanceListSupplier(ConfigurableApplicationContext context, RedissonClient redissonClient) {
		ServiceInstanceListSupplier delegate = ServiceInstanceListSupplier.builder()
				.withDiscoveryClient()
				.withCaching()
				.build(context);
		return new CustomServiceInstanceListSupplier(delegate, redissonClient);
	}
}

public class CustomServiceInstanceListSupplier extends DelegatingServiceInstanceListSupplier implements SelectedInstanceCallback {
...
}

When deploying the application to Kubernetes along spring-cloud-kubernetes-facric8-loadbalancer:2.1.4 and activating spring.cloud.kubernetes.loadbalancer.mode=SERVICE it's even getting worse

org.springframework.beans.factory.NoUniqueBeanDefinitionException: No qualifying bean of type 'org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier' available: more than one 'primary' bean found among candidates: [customServiceInstanceListSupplier, kubernetesServicesListSupplier, retryAwareDiscoveryClientServiceInstanceListSupplier]

Am I doing something wrong, or is the example from the post wrong?

[HJK181]

Removing @Primary leads to

Caused by: org.springframework.beans.factory.NoUniqueBeanDefinitionException: No qualifying bean of type 'org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier' available: expected single matching bean but found 2: customServiceInstanceListSupplier,kubernetesServicesListSupplier

[HJK181]

Further debugging revealed a second issue. My CustomServiceInstanceListSupplier implements SelectedInstanceCallback which the RoundRobinLoadBalancer respects by calling SelectedInstanceCallback#selectedServiceInstance

public class RoundRobinLoadBalancer implements ReactorServiceInstanceLoadBalancer {
...
private Response<ServiceInstance> processInstanceResponse(ServiceInstanceListSupplier supplier,
			List<ServiceInstance> serviceInstances) {
		Response<ServiceInstance> serviceInstanceResponse = getInstanceResponse(serviceInstances);
		if (supplier instanceof SelectedInstanceCallback && serviceInstanceResponse.hasServer()) {
			((SelectedInstanceCallback) supplier).selectedServiceInstance(serviceInstanceResponse.getServer());
		}
		return serviceInstanceResponse;
	}
}

However, with Spring retry on the classpath my CustomServiceInstanceListSupplier is wrapped in a RetryAwareServiceInstanceListSupplier which is not respecting SelectedInstanceCallback on its delegate.

@Configuration(proxyBeanMethods = false)
	@ConditionalOnBlockingDiscoveryEnabled
	@ConditionalOnClass(RetryTemplate.class)
	@Conditional(BlockingOnAvoidPreviousInstanceAndRetryEnabledCondition.class)
	@AutoConfigureAfter(BlockingSupportConfiguration.class)
	@ConditionalOnBean(ServiceInstanceListSupplier.class)
	public static class BlockingRetryConfiguration {

		@Bean
		@ConditionalOnBean(DiscoveryClient.class)
		@Primary
		public ServiceInstanceListSupplier retryAwareDiscoveryClientServiceInstanceListSupplier(
				ServiceInstanceListSupplier delegate) {
			return new RetryAwareServiceInstanceListSupplier(delegate);
		}

	}

I suspect that every implementation of ServiceInstanceListSupplier using some sort of "delegate" has the same problem, see RequestBasedStickySessionServiceInstanceListSupplier as an example.

[HJK181]

@OlgaMaciaszek any feedback appreciated as I'm stuck here. What I need to achieve is that my own ServiceInstanceListSupplier is used regardless of any auto-configured supplier. Or I need a way to use the auto-configured one as a delegate in my supplier, but that won't work as retryAwareDiscoveryClientServiceInstanceListSupplier is marked @Primary.

[HJK181]

@spencergibb Can you please have a look and give me hint if this is considered a bug or if any configuration possibility exists as a workaround? I've tried using @AutoConfigureAfter(LoadBalancerClientConfiguration.ReactiveRetryConfiguration.class) at my class annotated with @LoadBalancerClient without a chance. retryAwareDiscoveryClientServiceInstanceListSupplier is still configured after mine ...

[OlgaMaciaszek]

Hello, @HJK181. This is happening as for retrying failed requests, RetryAwareServiceInstanceListSupplier is enabled by default. As indicated in the docs, if you do not want it to be instantiated, you should set spring.cloud.loadbalancer.retry.avoidPreviousInstance to false. Does this solve your issue?

[HJK181]

I’d like to use the retry functionality, but in conjunction with my custom instance list supplier.

[OlgaMaciaszek]

That property does not disable retry. It only disables the RetryAwareServiceInstanceListSupplier that handles avoiding retrying on the same instance.

[HJK181]

Yeah but that’s a good thing I would prefer to not turn off. Particularly for all other load balancer clients in my spring cloud gateway (route based).

What I actually need is wrapping the RetryAwareServiceInstanceListSupplier in my custom supplier as a delegate but still have the primary RetryAwareServiceInstanceListSupplier for all other LB clients. Or as I noted above, all supplier that use a delegate should respect SelectedInstanceCallback on the delegate. This way my custom supplier can work as a delegate of RetryAwareServiceInstanceListSupplier

[OlgaMaciaszek]

@HJK181, that makes sense, however, the way this hierarchy has been designed is that the supplier provided by the user in a custom config (or our default one) is used as the delegate of the RetryAwareServiceInstanceListSupplier and not the other way around. Maybe you could rearchitect your solution to work in such a fashion? Since we haven't seen any similar issues, I don't think we'll be considering a change in this implementation at this point.

Alternatively, RetryAwareServiceInstanceListSupplier is public, so you might want to disable its automated instantiation via the property and then make your ServiceInstanceSupplier implementation extend it and execute your additional logic against the instance set returned after executing get() on the supertype.

[HJK181]

@OlgaMaciaszek Thank you for looking into it. Let me propose a solution that from my POV should fit into the current design while improving the overall functionality. The design allows one to create a custom configuration for an LB client and provide a custom implementation of ServiceInstanceListSupplier. However, the design does not correctly handle the case that the custom implementation implements SelectedInstanceCallback as the custom ServiceInstanceListSupplier might be used as a delegate, for example by RetryAwareServiceInstanceListSupplier.

To solve this the abstract DelegatingServiceInstanceListSupplier which is the base class of RetryAwareServiceInstanceListSupplier and hopefully all other ServiceInstanceListSupplier that use a delegate should just implement SelectedInstanceCallback and check if the delegate is an instanceof SelectedInstanceCallback like so

public abstract class DelegatingServiceInstanceListSupplier
		implements ServiceInstanceListSupplier, InitializingBean, DisposableBean, SelectedInstanceCallback {
....
@Override
public void selectedServiceInstance(ServiceInstance serviceInstance) {
  if (delegate instanceof SelectedInstanceCallback selectedInstanceCallbackDelegate) {
    selectedInstanceCallbackDelegate.selectedServiceInstance(serviceInstance);
  }
}
}

What do you think?

[OlgaMaciaszek]

@HJK181 That looks reasonable; we could put it in 4.1.x; would you like to submit a PR with that?

[HJK181]

I can check tomorrow if I find the time to setup my IDE for contributing to the project. Is there any chance that it will be backported to 3.1.x?

[OlgaMaciaszek]

Unfortunately, not really. We mostly only backport bugfixes. This would be a new feature.

[HJK181]

@OlgaMaciaszek #1226 FYI