From 272365490a175895ade5d73f038af9ac52b41fed Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Fri, 12 Mar 2021 12:55:44 +0100 Subject: [PATCH] Add hint-based instance filtering (#923) --- docs/src/main/asciidoc/_configprops.adoc | 1 + .../main/asciidoc/spring-cloud-commons.adoc | 34 ++++- .../loadbalancer/LoadBalancerProperties.java | 14 ++ .../HintBasedServiceInstanceListSupplier.java | 102 +++++++++++++ ...ckySessionServiceInstanceListSupplier.java | 5 - .../ServiceInstanceListSupplierBuilder.java | 9 ++ ...BasedServiceInstanceListSupplierTests.java | 141 ++++++++++++++++++ 7 files changed, 300 insertions(+), 6 deletions(-) create mode 100644 spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HintBasedServiceInstanceListSupplier.java create mode 100644 spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/HintBasedServiceInstanceListSupplierTests.java diff --git a/docs/src/main/asciidoc/_configprops.adoc b/docs/src/main/asciidoc/_configprops.adoc index e99a4ac1..68d83b6d 100644 --- a/docs/src/main/asciidoc/_configprops.adoc +++ b/docs/src/main/asciidoc/_configprops.adoc @@ -35,6 +35,7 @@ |spring.cloud.loadbalancer.health-check.refetch-instances-interval | `25s` | Interval for refetching available service instances. |spring.cloud.loadbalancer.health-check.repeat-health-check | `true` | Indicates whether health checks should keep repeating. It might be useful to set it to false if periodically refetching the instances, as every refetch will also trigger a healthcheck. |spring.cloud.loadbalancer.hint | | Allows setting the value of hint that is passed on to the LoadBalancer request and can subsequently be used in {@link ReactiveLoadBalancer} implementations. +|spring.cloud.loadbalancer.hint-header-name | `X-SC-LB-Hint` | Allows setting the name of the header used for passing the hint for hint-based service instance filtering. |spring.cloud.loadbalancer.retry.avoid-previous-instance | `true` | Enables wrapping ServiceInstanceListSupplier beans with `RetryAwareServiceInstanceListSupplier` if Spring-Retry is in the classpath. |spring.cloud.loadbalancer.retry.backoff.enabled | `false` | Indicates whether Reactor Retry backoffs should be applied. |spring.cloud.loadbalancer.retry.backoff.jitter | `0.5` | Used to set {@link RetryBackoffSpec#jitter}. diff --git a/docs/src/main/asciidoc/spring-cloud-commons.adoc b/docs/src/main/asciidoc/spring-cloud-commons.adoc index 70e225e6..93227711 100644 --- a/docs/src/main/asciidoc/spring-cloud-commons.adoc +++ b/docs/src/main/asciidoc/spring-cloud-commons.adoc @@ -932,7 +932,7 @@ public class CustomLoadBalancerConfiguration { @Bean public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier( ConfigurableApplicationContext context) { - return ServiceInstanceListSuppliers.builder() + return ServiceInstanceListSupplier.builder() .withDiscoveryClient() .withZonePreference() .withCaching() @@ -1053,6 +1053,38 @@ Spring Cloud LoadBalancer lets you set `String` hints that are passed to the Loa You can set a default hint for all services by setting the value of the `spring.cloud.loadbalancer.hint.default` property. You can also set a specific value for any given service by setting the value of the `spring.cloud.loadbalancer.hint.[SERVICE_ID]` property, substituting `[SERVICE_ID]` with the correct ID of your service. If the hint is not set by the user, `default` is used. +[[hints-based-loadbalancing]] +=== Hint-Based Load-Balancing + +We also provide a `HintBasedServiceInstanceListSupplier`, which is a `ServiceInstanceListSupplier` implementation for hint-based instance selection. + +`HintBasedServiceInstanceListSupplier` checks for a hint request header (the default header-name is `X-SC-LB-Hint`, but you can modify it by changing the value of the `spring.cloud.loadbalancer.hint-header-name` property) and, if it finds a hint request header, uses the hint value passed in the header to filter service instances. + +If no hint header has been added, `HintBasedServiceInstanceListSupplier` uses <> to filter service instances. + +If no hint is set, either by the header or by properties, all service instances provided by the delegate are returned. + +While filtering, `HintBasedServiceInstanceListSupplier` looks for service instances that have a matching value set under the `hint` key in their `metadataMap`. If no matching instances are found, all instances provided by the delegate are returned. + +You could use the following sample configuration to set it up: + +[[hints-based-custom-loadbalancer-configuration]] +[source,java,indent=0] +---- +public class CustomLoadBalancerConfiguration { + + @Bean + public ServiceInstanceListSupplier discoveryClientServiceInstanceListSupplier( + ConfigurableApplicationContext context) { + return ServiceInstanceListSupplier.builder() + .withDiscoveryClient() + .withHints() + .withCaching() + .build(context); + } +} +---- + [[spring-cloud-loadbalancer-starter]] === Spring Cloud LoadBalancer Starter diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java index 6400722c..bbefb715 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/LoadBalancerProperties.java @@ -49,6 +49,12 @@ public class LoadBalancerProperties { */ private Map hint = new LinkedCaseInsensitiveMap<>(); + /** + * Allows setting the name of the header used for passing the hint for hint-based + * service instance filtering. + */ + private String hintHeaderName = "X-SC-LB-Hint"; + /** * Properties for Spring-Retry and Reactor Retry support in Spring Cloud LoadBalancer. */ @@ -91,6 +97,14 @@ public class LoadBalancerProperties { this.stickySession = stickySession; } + public String getHintHeaderName() { + return hintHeaderName; + } + + public void setHintHeaderName(String hintHeaderName) { + this.hintHeaderName = hintHeaderName; + } + public static class StickySession { /** diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HintBasedServiceInstanceListSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HintBasedServiceInstanceListSupplier.java new file mode 100644 index 00000000..e13a487b --- /dev/null +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/HintBasedServiceInstanceListSupplier.java @@ -0,0 +1,102 @@ +/* + * Copyright 2012-2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import java.util.ArrayList; +import java.util.List; + +import reactor.core.publisher.Flux; + +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.HintRequestContext; +import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.Request; +import org.springframework.cloud.client.loadbalancer.RequestDataContext; +import org.springframework.http.HttpHeaders; +import org.springframework.util.StringUtils; + +/** + * A {@link ServiceInstanceListSupplier} implementation that uses hints to filter service + * instances provided by the delegate. + * + * @author Olga Maciaszek-Sharma + * @since 3.0.2 + */ +public class HintBasedServiceInstanceListSupplier extends DelegatingServiceInstanceListSupplier { + + private final LoadBalancerProperties properties; + + public HintBasedServiceInstanceListSupplier(ServiceInstanceListSupplier delegate, + LoadBalancerProperties properties) { + super(delegate); + this.properties = properties; + } + + @Override + public Flux> get() { + return delegate.get(); + } + + @Override + public Flux> get(Request request) { + return get().map(instances -> filteredByHint(instances, getHint(request.getContext()))); + } + + private String getHint(Object requestContext) { + if (requestContext == null) { + return null; + } + String hint = null; + if (requestContext instanceof RequestDataContext) { + hint = getHintFromHeader((RequestDataContext) requestContext); + } + if (!StringUtils.hasText(hint) && requestContext instanceof HintRequestContext) { + hint = ((HintRequestContext) requestContext).getHint(); + } + return hint; + } + + private String getHintFromHeader(RequestDataContext context) { + if (context.getClientRequest() != null) { + HttpHeaders headers = context.getClientRequest().getHeaders(); + if (headers != null) { + return headers.getFirst(properties.getHintHeaderName()); + } + } + return null; + } + + private List filteredByHint(List instances, String hint) { + if (!StringUtils.hasText(hint)) { + return instances; + } + List filteredInstances = new ArrayList<>(); + for (ServiceInstance serviceInstance : instances) { + if (serviceInstance.getMetadata().getOrDefault("hint", "").equals(hint)) { + filteredInstances.add(serviceInstance); + } + } + if (filteredInstances.size() > 0) { + return filteredInstances; + } + + // If instances cannot be found based on hint, + // we return all instances retrieved for given service id. + return instances; + } + +} diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RequestBasedStickySessionServiceInstanceListSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RequestBasedStickySessionServiceInstanceListSupplier.java index 1c95b2a2..83e42433 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RequestBasedStickySessionServiceInstanceListSupplier.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RequestBasedStickySessionServiceInstanceListSupplier.java @@ -48,11 +48,6 @@ public class RequestBasedStickySessionServiceInstanceListSupplier extends Delega this.properties = properties; } - @Override - public String getServiceId() { - return delegate.getServiceId(); - } - @Override public Flux> get() { return delegate.get(); diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java index 39d20fb2..02843f6c 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/ServiceInstanceListSupplierBuilder.java @@ -239,6 +239,15 @@ public final class ServiceInstanceListSupplierBuilder { return this; } + public ServiceInstanceListSupplierBuilder withHints() { + DelegateCreator creator = (context, delegate) -> { + LoadBalancerProperties properties = context.getBean(LoadBalancerProperties.class); + return new HintBasedServiceInstanceListSupplier(delegate, properties); + }; + creators.add(creator); + return this; + } + /** * Builds the {@link ServiceInstanceListSupplier} hierarchy. * @param context application context diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/HintBasedServiceInstanceListSupplierTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/HintBasedServiceInstanceListSupplierTests.java new file mode 100644 index 00000000..0f4d5c12 --- /dev/null +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/HintBasedServiceInstanceListSupplierTests.java @@ -0,0 +1,141 @@ +/* + * Copyright 2012-2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.loadbalancer.core; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Flux; + +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.DefaultRequest; +import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.Request; +import org.springframework.cloud.client.loadbalancer.RequestData; +import org.springframework.cloud.client.loadbalancer.RequestDataContext; +import org.springframework.mock.http.client.MockClientHttpRequest; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Tests for {@link HintBasedServiceInstanceListSupplier}. + * + * @author Olga Maciaszek-Sharma + */ +class HintBasedServiceInstanceListSupplierTests { + + private final DiscoveryClientServiceInstanceListSupplier delegate = mock( + DiscoveryClientServiceInstanceListSupplier.class); + + private final LoadBalancerProperties properties = new LoadBalancerProperties(); + + private final RequestDataContext requestContext = new RequestDataContext( + new RequestData(new MockClientHttpRequest())); + + private final HintBasedServiceInstanceListSupplier supplier = new HintBasedServiceInstanceListSupplier(delegate, + properties); + + private final ServiceInstance first = serviceInstance("test-1", buildHintMetadata("test1")); + + private final ServiceInstance second = serviceInstance("test-2", buildHintMetadata("test2")); + + private final ServiceInstance third = serviceInstance("test-3", new HashMap<>()); + + @BeforeEach + void setUp() { + properties.setHintHeaderName("X-Test"); + when(delegate.get()).thenReturn(Flux.just(Arrays.asList(first, second, third))); + } + + @Test + void shouldReturnInstancesForHintFromHeaderWhenAvailable() { + requestContext.setHint("test1"); + requestContext.getClientRequest().getHeaders().add("X-Test", "test2"); + Request request = new DefaultRequest<>(requestContext); + + List filtered = supplier.get(request).blockFirst(); + + assertThat(filtered).hasSize(1); + assertThat(filtered.get(0).getInstanceId()).isEqualTo("test-2"); + } + + @Test + void shouldReturnInstancesForHintFromPropertiesWhenNoHintHeader() { + requestContext.setHint("test1"); + Request request = new DefaultRequest<>(requestContext); + + List filtered = supplier.get(request).blockFirst(); + + assertThat(filtered).hasSize(1); + assertThat(filtered.get(0).getInstanceId()).isEqualTo("test-1"); + } + + @Test + void shouldReturnAllInstancesWhenNoHint() { + Request request = new DefaultRequest<>(requestContext); + + List filtered = supplier.get(request).blockFirst(); + + assertThat(filtered).hasSize(3); + } + + @Test + void shouldReturnAllInstancesWhenHintNotMatched() { + requestContext.getClientRequest().getHeaders().add("X-Test", "testX"); + Request request = new DefaultRequest<>(requestContext); + + List filtered = supplier.get(request).blockFirst(); + + assertThat(filtered).hasSize(3); + } + + @Test + void shouldReturnAllInstancesWhenRequestContextNull() { + Request request = new DefaultRequest<>(null); + + List filtered = supplier.get(request).blockFirst(); + + assertThat(filtered).hasSize(3); + } + + @Test + void shouldReturnAllInstancesWhenClientRequestNull() { + Request request = new DefaultRequest<>(new RequestDataContext(null)); + + List filtered = supplier.get(request).blockFirst(); + + assertThat(filtered).hasSize(3); + } + + private DefaultServiceInstance serviceInstance(String instanceId, Map metadata) { + return new DefaultServiceInstance(instanceId, "test", "http://test.test", 9080, false, metadata); + } + + private Map buildHintMetadata(String zone) { + Map metadata = new HashMap<>(); + metadata.put("hint", zone); + return metadata; + } + +}