Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LoadBalancer: support hint-based instance selection #672

Closed
OlgaMaciaszek opened this issue Jan 16, 2020 · 8 comments · Fixed by #923
Closed

LoadBalancer: support hint-based instance selection #672

OlgaMaciaszek opened this issue Jan 16, 2020 · 8 comments · Fixed by #923
Assignees
Milestone

Comments

@OlgaMaciaszek
Copy link
Collaborator

OlgaMaciaszek commented Jan 16, 2020

Related to: #647

Possible implementation:

  • a request comes with loadBalancerHint header = someVal
  • based on that header, we would pass the request with hint loadBalancer.choose(serviceId, request), where someVal can be retrieved from request.getContext()
  • then filter instances by metadataMap.get("hint").equals("someVal")
@andersenleo
Copy link

andersenleo commented Mar 31, 2020

Hi -
we're currently migrating our code from the Ribbon load-balancer to Spring-Cloud. We've started hacking up a custom load-balancer (based on ReactorServiceInstanceLoadBalancer and fallback to the RoundRobinLoadBalancer).

Our requirements is that we need to query service-instances to check if they own a specific "key" and select the one that holds the key. We're planning to create our own interceptor/filter for RestTemplate and WebClient to propagate the "key" from a http-header or request-attribute (and create a Request with a context that is the "key"). This is more or less a carbon-copy of the existing ReactorLoadBalancerExchangeFilterFunction (also for RestTemplate - the LoadBalancerClient abstraction isn't really suitable for us).

Now - we can probably work our way around the current Spring-Cloud LoadBalancer code, but it would be nice if we could either align with upcoming changes to the your code -- or even better, if we could contribute our code to Spring-Cloud :).

So - to summarise: What's the current status on this issue? Are there any design docs or discussions that we could read up on to get an idea about what direction you're heading?

@OlgaMaciaszek
Copy link
Collaborator Author

@andersenleo would definitely be great if you could provide a PR - we have this in the backlog for 2020-1 release train (Ilford), but it might be released faster if you contribute. Let me get back to you tomorrow here with some ideas on how we planned to implement it and let's discuss it then.

@OlgaMaciaszek
Copy link
Collaborator Author

We've been thinking about the following features:

  • A request would come with a specific header, for example loadBalancerHint
  • We would be able to retrieve it within ReactorLoadBalancerFilterFunction and use org.springframework.cloud.client.loadbalancer.reactive.Request and its getContext() method to pass that data forward to the LoadBalancer. There's the Publisher<Response<T>> choose(Request request) method, but also the <R, C> Publisher<R> execute(RequestExecution<R, C, T> execution); method has recently been added and we are planning on switching to using it as part of this feature
  • The hint-based loadBalancer implementation would filter the service instances containing the specified value under the loadBalancerHint key in the ServiceInstance metadataMap
  • There should be a possibility to modify the hint header/ key name via properties (Maybe allow SpEL expressions)
  • There should be a possibility to pass the key-value pair to look for in the metadata via properties instead of the request header as well (I guess if the request header is passed, that should take precedence, but we should also probably allow changing it via a property)

These are the ideas we've had, but it's still a good time to have a discussion about it and propose alternative design solutions, so feel free to pass your suggestions.

Note: I will be working on #675 this and probably next week, so it might make sense for you to wait till that's merged, as it will require switching to the loadBalancer exectue(...) method, anyway.

@andersenleo
Copy link

Thanks a lot for the information, @OlgaMaciaszek
We've approached the solution in a similar way as you described, but with some significant difference wrt ServiceInstance filtering. It might be unique to our use-case though. We'll try to make a more detailed write-up of our solution as soon as possible (and post it here).

@m11r9000
Copy link

m11r9000 commented Apr 9, 2020

Hi @OlgaMaciaszek and thank you for the info.

One significant difference is that the process of filtering as you suggest is static as it only looks at the metadata of the service instance, whereas we actually hit the services to see, according to pre-defined conditions, which one is to be chosen. Your suggested strategy of checking the service instance meta-data could be a default strategy, but we should maybe have the option of plugging in other strategies? More about that below ⬇️

As promised, here is the write-up for how we handle service instance filtering:

  • We have created a class that inherits the ReactorLoadBalancerFilterFunction to extract the hint from the header of the incoming org.springframework.web.reactive.function.client.ClientRequest. The hint is used to create a org.springframework.cloud.client.loadbalancer.reactive.Request by embedding it inside the context object of the Request

  • The filter method then returns the value given by the command loadbalancer.choose(requestWithHint). The loadbalancer has been previously selected via the ReactiveLoadBalancer.Factory<ServiceInstance>.

  • loadbalancer#choose(Request) is invoked in our custom load balancer. This load balancer makes use of a pluggable implementation called ServiceInstanceSelector defined as

public interface ServiceInstanceSelector {
        Mono<ServiceInstance> select(Object hint, Flux<ServiceInstance> instances);
}
  • Basically, given a set of service instances for a service endpoint and a hint, select the appropriate service instance.

  • To do this our implementation of the ServiceInstanceSelector makes use of:

    • a request factory HttpQueryFactory<Object, RequestEntity<Object>> used to generate a RequestEntity specifying what endpoint on the service instances we will be evaluating.
    • a response evaluator HttpResponseCheck<ResponseEntity<Object>> that takes the response from the executed request and evaluates the response according to a specified predicate.
    • With all of these components in place we have a pluggable implementation that can be used to hit a service instance and check if it's valid according to a pre-defined condition. Here is an example bean of the ServiceInstanceSelector:
@Bean
public Interfaces.ServiceInstanceSelector serviceInstanceSelector() {
            Interfaces.HttpQueryFactory<Object, RequestEntity<Object>> requestFactory = (hint, uri) -> {
                URI target = UriComponentsBuilder.fromUri(uri)
                        .path("/check")
                        .queryParam("id", hint)
                        .build().toUri();
                return new RequestEntity<>(HttpMethod.GET, target);
            };

            Interfaces.HttpResponseCheck<ResponseEntity<Object>> evaluator = 
           resp -> resp.getStatusCode().is2xxSuccessful();

            return new HttpCheckInstanceSelector(requestFactory, evaluator);
}
  • The HttpCheckInstanceSelector is an implementation of ServiceinstanceSelector responsible for actually executing aforementioned request for a set of service instances, evaluating the responses, and returning the first service instance found which had a response that evaluated to true. If you're curious, here is the most essential piece of code performed in HttpCheckInstanceSelector:
@Override
public Mono<ServiceInstance> select(Object hint, Flux<ServiceInstance> candidates) {
    return candidates
            .filter((si) ->
                    evaluator.test(restTemplate.exchange(requestFactory.create(hint, si.getUri()),
                                Object.class)))
            .next();
}
  • Finally, the custom load balancer works like this:
    • check if a ServiceInstanceSelector instance is available.
    • check if the Request object contains a context (that should contain a hint)
    • If no context and selector, fall back to round robin strategy
    • otherwise, fetch a Flux for the specific serviceId and extract a Mono<Response> by invoking ServiceInstanceSelector#select and return this Mono.

If you're curious the #choose method is specified below:

public Mono<Response<ServiceInstance>> choose(Request request) {
        Interfaces.ServiceInstanceSelector selector = this.selector.getIfAvailable();
        Object context = request.getContext();

        if (context != null && selector != null) {
            log.info("Service locator and balance key available - selecting appropriate instance");
            Flux<ServiceInstance> candidates = Flux.fromStream(supplier.stream()
                    .filter(s -> s.getServiceId().equals(serviceId))
                    .flatMap(s -> s.get().toStream())
                    .flatMap(Collection::stream));

            Mono<Response<ServiceInstance>> response = selector
                    .select(context, candidates)
                    .map(DefaultResponse::new);
            return response.switchIfEmpty(fallbackLoadbalancer.choose(request));
        } else {
            return fallbackLoadbalancer.choose(request);
        }
    }

Regarding your strategy for checking metadata on the service instances, that can be implemented with the ServiceInstanceSelector like this:

public class MetedataInstanceSelector implements Interfaces.ServiceInstanceSelector {

    @Override
    public Mono<ServiceInstance> select(Object metadataValue, Flux<ServiceInstance> candidates) {
        return candidates
                .filter((si) -> si.getMetadata().getOrDefault("loadBalancerHint", "NO_HINT")
                        .equals(metadataValue))
                .next();
    }
}

Furthermore we could even export the evaluation to maybe a BiPredicate in order for developers to point on different key/values and not just "loadBalancerHint", but hopefully the intention is clear in this example.

We would love your thoughts on this!

@OlgaMaciaszek
Copy link
Collaborator Author

* We have created a class that inherits the `ReactorLoadBalancerFilterFunction` to extract the hint from the header of the incoming `org.springframework.web.reactive.function.client.ClientRequest`. The hint is used to create a `org.springframework.cloud.client.loadbalancer.reactive.Request` by embedding it inside the context object of the `Request` 

I suggest you just make and contribute changes in the ReactorLoadBalancerFilterFunction to support that and just pass it always (we already have pluggable ReactiveLoadBalancer and ServiceInstanceListSupplier implementations, so I would probably and that should be enough for most of the use-cases without having to work with multiple ReactorLoadBalancerFilterFunction implementations); @spencergibb wdyt?

I think we would like to stick with the metadata-based hint implementation for now (at least until we see more users that would like to use a dynamic one in future) and you could use the one you describe as your own custom implementation (I can implement the metadata-based one once you've contributed the changes to the exchange filter function).

@m11r9000
Copy link

m11r9000 commented Apr 11, 2020

Hi @OlgaMaciaszek . Sorry for the late reply. Thank you for the input and I'll look at sending over a PR for ReactorLoadBalancerFilterFunction and possibly also for coming back with some more questions. Do note that Monday is a public holiday here in Sweden so you'll probably start seeing some activity on Tuesday.

@andersenleo
Copy link

I think you're right about not introducing the ServiceInstanceSelector as a core abstraction in Spring-Cloud @OlgaMaciaszek --- we have a pretty specific use-case, and doing something like "picking one or more service-instances based on some logic" is the core of load-balancing, so it belongs in a custom "LoadBalancer" 👍

We'll try to iron out a PR (me and @andaziar) with the intention/scope of allowing @LoadBalanced RestTemplate/WebClient components to transform an HTTP call into a Request<T> to the underlying load-balancer (where the default strategy would be to extract a "hint" from the request and pass that along as a request context).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants