Skip to content

Commit 354a894

Browse files
OlgaMaciaszekOlgaMaciaszek
authored
Call get request on delegates (#1250)
1 parent 31c07aa commit 354a894

File tree

11 files changed

+200
-47
lines changed

11 files changed

+200
-47
lines changed

‎docs/src/main/asciidoc/spring-cloud-commons.adoc

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,11 @@ to `false`.
933933

934934
WARNING: Although the basic, non-cached, implementation is useful for prototyping and testing, it's much less efficient than the cached versions, so we recommend always using the cached version in production. If the caching is already done by the `DiscoveryClient` implementation, for example `EurekaDiscoveryClient`, the load-balancer caching should be disabled to prevent double caching.
935935

936+
====
937+
938+
NOTE: When you create your own configuration, if you use `CachingServiceInstanceListSupplier` make sure to place it in the hierarchy directly after the supplier that retrieves the instances over the network, for example, `DiscoveryClientServiceInstanceListSupplier`, before any other filtering suppliers.
939+
940+
====
936941
=== Zone-Based Load-Balancing
937942

938943
To enable zone-based load-balancing, we provide the `ZonePreferenceServiceInstanceListSupplier`.
@@ -950,7 +955,7 @@ If the zone is `null` or there are no instances within the same zone, it returns
950955
In order to use the zone-based load-balancing approach, you will have to instantiate a `ZonePreferenceServiceInstanceListSupplier` bean in a <<custom-loadbalancer-configuration,custom configuration>>.
951956

952957
We use delegates to work with `ServiceInstanceListSupplier` beans.
953-
We suggest passing a `DiscoveryClientServiceInstanceListSupplier` delegate in the constructor of `ZonePreferenceServiceInstanceListSupplier` and, in turn, wrapping the latter with a `CachingServiceInstanceListSupplier` to leverage <<loadbalancer-caching, LoadBalancer caching mechanism>>.
958+
We suggest using a `DiscoveryClientServiceInstanceListSupplier` delegate, wrapping it with a `CachingServiceInstanceListSupplier` to leverage <<loadbalancer-caching, LoadBalancer caching mechanism>>, and then passing the resulting bean in the constructor of `ZonePreferenceServiceInstanceListSupplier`.
954959

955960
You can use this sample configuration to set it up:
956961

@@ -964,8 +969,8 @@ public class CustomLoadBalancerConfiguration {
964969
ConfigurableApplicationContext context) {
965970
return ServiceInstanceListSupplier.builder()
966971
.withDiscoveryClient()
972+
.withCaching()
967973
.withZonePreference()
968-
.withCaching()
969974
.build(context);
970975
}
971976
}
@@ -1026,6 +1031,12 @@ You can also pass your own `WebClient` or `RestTemplate` instance to be used for
10261031

10271032
WARNING: `HealthCheckServiceInstanceListSupplier` has its own caching mechanism based on Reactor Flux `replay()`. Therefore, if it's being used, you may want to skip wrapping that supplier with `CachingServiceInstanceListSupplier`.
10281033

1034+
====
1035+
1036+
NOTE: When you create your own configuration, `HealthCheckServiceInstanceListSupplier`, make sure to place it in the hierarchy directly after the supplier that retrieves the instances over the network, for example, `DiscoveryClientServiceInstanceListSupplier`, before any other filtering suppliers.
1037+
1038+
====
1039+
10291040
=== Same instance preference for LoadBalancer
10301041

10311042
You can set up the LoadBalancer in such a way that it prefers the instance that was previously selected, if that instance is available.
@@ -1110,8 +1121,8 @@ public class CustomLoadBalancerConfiguration {
11101121
ConfigurableApplicationContext context) {
11111122
return ServiceInstanceListSupplier.builder()
11121123
.withDiscoveryClient()
1124+
.withCaching()
11131125
.withHints()
1114-
.withCaching()
11151126
.build(context);
11161127
}
11171128
}
@@ -1221,11 +1232,18 @@ public class MyConfiguration {
12211232
}
12221233
}
12231234
----
1235+
====
12241236

12251237
NOTE: The classes you pass as `@LoadBalancerClient` or `@LoadBalancerClients` configuration arguments should either not be annotated with `@Configuration` or be outside component scan scope.
12261238

12271239
====
12281240
1241+
====
1242+
1243+
NOTE: When you create your own configuration, if you use `CachingServiceInstanceListSupplier` or `HealthCheckServiceInstanceListSupplier`, makes sure to use one of them, not both, and make sure to place it in the hierarchy directly after the supplier that retrieves the instances over the network, for example, `DiscoveryClientServiceInstanceListSupplier`, before any other filtering suppliers.
1244+
1245+
====
1246+
12291247
[[loadbalancer-lifecycle]]
12301248
=== Spring Cloud LoadBalancer Lifecycle
12311249
@@ -1301,6 +1319,8 @@ The per-client configuration properties work for most of the properties, apart f
13011319
13021320
NOTE: For the properties where maps where already used, where you can specify a different value per-client without using the `clients` keyword (for example, `hints`, `health-check.path`), we have kept that behaviour in order to keep the library backwards compatible. It will be modified in the next major release.
13031321
1322+
NOTE: Starting with `3.1.7` in `2021.0.x` release train, `4.0.4` in `2022.0.x` release train and `4.1.0` in the `2023.0.x` release train, we have introduced the `callGetWithRequestOnDelegates` flag in `LoadBalancerProperties`. If this flag is set to `true`, `ServiceInstanceListSupplier#get(Request request)` method will be implemented to call `delegate.get(request)` in classes assignable from `DelegatingServiceInstanceListSupplier` that don't already implement that method, with the exclusion of `CachingServiceInstanceListSupplier` and `HealthCheckServiceInstanceListSupplier`, which should be placed in the instance supplier hierarchy directly after the supplier performing instance retrieval over the network, before any request-based filtering is done. For `3.1.x` and `4.0.x` the flag is set to `false` by default, and since `4.1.0` it's going to be set to `true` by default.
1323+
13041324
== Spring Cloud Circuit Breaker
13051325
13061326
include::spring-cloud-circuitbreaker.adoc[leveloffset=+1]

‎spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,19 @@ public class LoadBalancerProperties {
7373
*/
7474
private boolean useRawStatusCodeInResponseData;
7575

76+
/**
77+
* If this flag is set to {@code true},
78+
* {@code ServiceInstanceListSupplier#get(Request request)} method will be implemented
79+
* to call {@code delegate.get(request)} in classes assignable from
80+
* {@code DelegatingServiceInstanceListSupplier} that don't already implement that
81+
* method, with the exclusion of {@code CachingServiceInstanceListSupplier} and
82+
* {@code HealthCheckServiceInstanceListSupplier}, which should be placed in the
83+
* instance supplier hierarchy directly after the supplier performing instance
84+
* retrieval over the network, before any request-based filtering is done. Note: in
85+
* 4.1, this behaviour will become the default
86+
*/
87+
private boolean callGetWithRequestOnDelegates;
88+
7689
public HealthCheck getHealthCheck() {
7790
return healthCheck;
7891
}
@@ -134,6 +147,36 @@ public void setUseRawStatusCodeInResponseData(boolean useRawStatusCodeInResponse
134147
this.useRawStatusCodeInResponseData = useRawStatusCodeInResponseData;
135148
}
136149

150+
/**
151+
* If this flag is set to {@code true},
152+
* {@code ServiceInstanceListSupplier#get(Request request)} method will be implemented
153+
* to call {@code delegate.get(request)} in classes assignable from
154+
* {@code DelegatingServiceInstanceListSupplier} that don't already implement that
155+
* method, with the exclusion of {@code CachingServiceInstanceListSupplier} and
156+
* {@code HealthCheckServiceInstanceListSupplier}, which should be placed in the
157+
* instance supplier hierarchy directly after the supplier performing instance
158+
* retrieval over the network, before any request-based filtering is done. Note: in
159+
* 4.1, this behaviour will become the default
160+
*/
161+
public boolean isCallGetWithRequestOnDelegates() {
162+
return callGetWithRequestOnDelegates;
163+
}
164+
165+
/**
166+
* If this flag is set to {@code true},
167+
* {@code ServiceInstanceListSupplier#get(Request request)} method will be implemented
168+
* to call {@code delegate.get(request)} in classes assignable from
169+
* {@code DelegatingServiceInstanceListSupplier} that don't already implement that
170+
* method, with the exclusion of {@code CachingServiceInstanceListSupplier} and
171+
* {@code HealthCheckServiceInstanceListSupplier}, which should be placed in the
172+
* instance supplier hierarchy directly after the supplier performing instance
173+
* retrieval over the network, before any request-based filtering is done. Note: in
174+
* 4.1, this behaviour will become the default
175+
*/
176+
public void setCallGetWithRequestOnDelegates(boolean callGetWithRequestOnDelegates) {
177+
this.callGetWithRequestOnDelegates = callGetWithRequestOnDelegates;
178+
}
179+
137180
public static class StickySession {
138181

139182
/**

‎spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/annotation/LoadBalancerClientConfiguration.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier(
9393
@Conditional(ZonePreferenceConfigurationCondition.class)
9494
public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceListSupplier(
9595
ConfigurableApplicationContext context) {
96-
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withZonePreference().withCaching()
96+
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withCaching().withZonePreference()
9797
.build(context);
9898
}
9999

@@ -119,8 +119,8 @@ public ServiceInstanceListSupplier healthCheckDiscoveryClientServiceInstanceList
119119
@Conditional(RequestBasedStickySessionConfigurationCondition.class)
120120
public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServiceInstanceListSupplier(
121121
ConfigurableApplicationContext context) {
122-
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withRequestBasedStickySession()
123-
.withCaching().build(context);
122+
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withCaching()
123+
.withRequestBasedStickySession().build(context);
124124
}
125125

126126
@Bean
@@ -129,8 +129,8 @@ public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServi
129129
@Conditional(SameInstancePreferenceConfigurationCondition.class)
130130
public ServiceInstanceListSupplier sameInstancePreferenceServiceInstanceListSupplier(
131131
ConfigurableApplicationContext context) {
132-
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withSameInstancePreference()
133-
.withCaching().build(context);
132+
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withCaching()
133+
.withSameInstancePreference().build(context);
134134
}
135135

136136
}
@@ -155,8 +155,8 @@ public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier(
155155
@Conditional(ZonePreferenceConfigurationCondition.class)
156156
public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceListSupplier(
157157
ConfigurableApplicationContext context) {
158-
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withZonePreference()
159-
.withCaching().build(context);
158+
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withCaching()
159+
.withZonePreference().build(context);
160160
}
161161

162162
@Bean
@@ -175,8 +175,8 @@ public ServiceInstanceListSupplier healthCheckDiscoveryClientServiceInstanceList
175175
@Conditional(RequestBasedStickySessionConfigurationCondition.class)
176176
public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServiceInstanceListSupplier(
177177
ConfigurableApplicationContext context) {
178-
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withRequestBasedStickySession()
179-
.withCaching().build(context);
178+
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withCaching()
179+
.withRequestBasedStickySession().build(context);
180180
}
181181

182182
@Bean
@@ -185,8 +185,8 @@ public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServi
185185
@Conditional(SameInstancePreferenceConfigurationCondition.class)
186186
public ServiceInstanceListSupplier sameInstancePreferenceServiceInstanceListSupplier(
187187
ConfigurableApplicationContext context) {
188-
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withSameInstancePreference()
189-
.withCaching().build(context);
188+
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withCaching()
189+
.withSameInstancePreference().build(context);
190190
}
191191

192192
}

‎spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/DelegatingServiceInstanceListSupplier.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ public DelegatingServiceInstanceListSupplier(ServiceInstanceListSupplier delegat
3838
}
3939

4040
public ServiceInstanceListSupplier getDelegate() {
41-
return this.delegate;
41+
return delegate;
4242
}
4343

4444
@Override
4545
public String getServiceId() {
46-
return this.delegate.getServiceId();
46+
return delegate.getServiceId();
4747
}
4848

4949
@Override

‎spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/SameInstancePreferenceServiceInstanceListSupplier.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import reactor.core.publisher.Flux;
2525

2626
import org.springframework.cloud.client.ServiceInstance;
27+
import org.springframework.cloud.client.loadbalancer.Request;
28+
import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer;
2729

2830
/**
2931
* An implementation of {@link ServiceInstanceListSupplier} that selects the previously
@@ -39,10 +41,19 @@ public class SameInstancePreferenceServiceInstanceListSupplier extends Delegatin
3941

4042
private ServiceInstance previouslyReturnedInstance;
4143

44+
private boolean callGetWithRequestOnDelegates;
45+
4246
public SameInstancePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate) {
4347
super(delegate);
4448
}
4549

50+
public SameInstancePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate,
51+
ReactiveLoadBalancer.Factory<ServiceInstance> loadBalancerClientFactory) {
52+
super(delegate);
53+
callGetWithRequestOnDelegates = loadBalancerClientFactory.getProperties(getServiceId())
54+
.isCallGetWithRequestOnDelegates();
55+
}
56+
4657
@Override
4758
public String getServiceId() {
4859
return delegate.getServiceId();
@@ -53,6 +64,14 @@ public Flux<List<ServiceInstance>> get() {
5364
return delegate.get().map(this::filteredBySameInstancePreference);
5465
}
5566

67+
@Override
68+
public Flux<List<ServiceInstance>> get(Request request) {
69+
if (callGetWithRequestOnDelegates) {
70+
return delegate.get(request).map(this::filteredBySameInstancePreference);
71+
}
72+
return get();
73+
}
74+
5675
private List<ServiceInstance> filteredBySameInstancePreference(List<ServiceInstance> serviceInstances) {
5776
if (previouslyReturnedInstance != null && serviceInstances.contains(previouslyReturnedInstance)) {
5877
if (LOG.isDebugEnabled()) {

‎spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,6 @@ public final class ServiceInstanceListSupplierBuilder {
5757

5858
private Creator baseCreator;
5959

60-
private DelegateCreator cachingCreator;
61-
6260
private final List<DelegateCreator> creators = new ArrayList<>();
6361

6462
ServiceInstanceListSupplierBuilder() {
@@ -148,8 +146,10 @@ public ServiceInstanceListSupplierBuilder withHealthChecks(WebClient webClient)
148146
* @return the {@link ServiceInstanceListSupplierBuilder} object
149147
*/
150148
public ServiceInstanceListSupplierBuilder withSameInstancePreference() {
151-
DelegateCreator creator = (context,
152-
delegate) -> new SameInstancePreferenceServiceInstanceListSupplier(delegate);
149+
DelegateCreator creator = (context, delegate) -> {
150+
LoadBalancerClientFactory loadBalancerClientFactory = context.getBean(LoadBalancerClientFactory.class);
151+
return new SameInstancePreferenceServiceInstanceListSupplier(delegate, loadBalancerClientFactory);
152+
};
153153
this.creators.add(creator);
154154
return this;
155155
}
@@ -191,8 +191,9 @@ public ServiceInstanceListSupplierBuilder withBlockingHealthChecks(RestTemplate
191191
*/
192192
public ServiceInstanceListSupplierBuilder withZonePreference() {
193193
DelegateCreator creator = (context, delegate) -> {
194+
LoadBalancerClientFactory loadBalancerClientFactory = context.getBean(LoadBalancerClientFactory.class);
194195
LoadBalancerZoneConfig zoneConfig = context.getBean(LoadBalancerZoneConfig.class);
195-
return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig);
196+
return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig, loadBalancerClientFactory);
196197
};
197198
this.creators.add(creator);
198199
return this;
@@ -206,8 +207,9 @@ public ServiceInstanceListSupplierBuilder withZonePreference() {
206207
*/
207208
public ServiceInstanceListSupplierBuilder withZonePreference(String zoneName) {
208209
DelegateCreator creator = (context, delegate) -> {
210+
LoadBalancerClientFactory loadBalancerClientFactory = context.getBean(LoadBalancerClientFactory.class);
209211
LoadBalancerZoneConfig zoneConfig = new LoadBalancerZoneConfig(zoneName);
210-
return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig);
212+
return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig, loadBalancerClientFactory);
211213
};
212214
this.creators.add(creator);
213215
return this;
@@ -228,19 +230,15 @@ public ServiceInstanceListSupplierBuilder withRequestBasedStickySession() {
228230
}
229231

230232
/**
231-
* If {@link LoadBalancerCacheManager} is available in the context, wraps created
232-
* {@link ServiceInstanceListSupplier} hierarchy with a
233-
* {@link CachingServiceInstanceListSupplier} instance to provide a caching mechanism
234-
* for service instances. Uses {@link ObjectProvider} to lazily resolve
233+
* If {@link LoadBalancerCacheManager} is available in the context, adds a
234+
* {@link CachingServiceInstanceListSupplier} instance to the
235+
* {@link ServiceInstanceListSupplier} hierarchy to provide a caching mechanism for
236+
* service instances. Uses {@link ObjectProvider} to lazily resolve
235237
* {@link LoadBalancerCacheManager}.
236238
* @return the {@link ServiceInstanceListSupplierBuilder} object
237239
*/
238240
public ServiceInstanceListSupplierBuilder withCaching() {
239-
if (cachingCreator != null && LOG.isWarnEnabled()) {
240-
LOG.warn(
241-
"Overriding a previously set cachingCreator with a CachingServiceInstanceListSupplier-based cachingCreator.");
242-
}
243-
this.cachingCreator = (context, delegate) -> {
241+
DelegateCreator creator = (context, delegate) -> {
244242
ObjectProvider<LoadBalancerCacheManager> cacheManagerProvider = context
245243
.getBeanProvider(LoadBalancerCacheManager.class);
246244
if (cacheManagerProvider.getIfAvailable() != null) {
@@ -251,6 +249,7 @@ public ServiceInstanceListSupplierBuilder withCaching() {
251249
}
252250
return delegate;
253251
};
252+
creators.add(creator);
254253
return this;
255254
}
256255

@@ -297,9 +296,6 @@ public ServiceInstanceListSupplier build(ConfigurableApplicationContext context)
297296
supplier = creator.apply(context, supplier);
298297
}
299298

300-
if (this.cachingCreator != null) {
301-
supplier = this.cachingCreator.apply(context, supplier);
302-
}
303299
return supplier;
304300
}
305301

‎spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ZonePreferenceServiceInstanceListSupplier.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import reactor.core.publisher.Flux;
2424

2525
import org.springframework.cloud.client.ServiceInstance;
26+
import org.springframework.cloud.client.loadbalancer.Request;
27+
import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer;
2628
import org.springframework.cloud.loadbalancer.config.LoadBalancerZoneConfig;
2729

2830
/**
@@ -43,17 +45,36 @@ public class ZonePreferenceServiceInstanceListSupplier extends DelegatingService
4345

4446
private String zone;
4547

48+
private boolean callGetWithRequestOnDelegates;
49+
4650
public ZonePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate,
4751
LoadBalancerZoneConfig zoneConfig) {
4852
super(delegate);
4953
this.zoneConfig = zoneConfig;
5054
}
5155

56+
public ZonePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate,
57+
LoadBalancerZoneConfig zoneConfig,
58+
ReactiveLoadBalancer.Factory<ServiceInstance> loadBalancerClientFactory) {
59+
super(delegate);
60+
this.zoneConfig = zoneConfig;
61+
callGetWithRequestOnDelegates = loadBalancerClientFactory.getProperties(getServiceId())
62+
.isCallGetWithRequestOnDelegates();
63+
}
64+
5265
@Override
5366
public Flux<List<ServiceInstance>> get() {
5467
return getDelegate().get().map(this::filteredByZone);
5568
}
5669

70+
@Override
71+
public Flux<List<ServiceInstance>> get(Request request) {
72+
if (callGetWithRequestOnDelegates) {
73+
return getDelegate().get(request).map(this::filteredByZone);
74+
}
75+
return get();
76+
}
77+
5778
private List<ServiceInstance> filteredByZone(List<ServiceInstance> serviceInstances) {
5879
if (zone == null) {
5980
zone = zoneConfig.getZone();

0 commit comments

Comments
 (0)