POC for a session/cookie based sitcky load balancer implementation.

[fitzoh]

Possible implementation for #689, meant as a discussion starter (no tests, not wired up to anything).

How it works

The StickySessionLoadBalancer looks for a server id in the scg-instance-id cookie (not married to that name).

If a cookie value is found and that value is a valid service instance ID, the request is routed to that service instance.

If it is not found or if the value is invalid (perhaps the server is no longer active or unhealthy), then a delegate ReactorServiceInstanceLoadBalancer and the request is routed to that server instead.

Regardless of the path taken, the cookie is updated with the instance ID that the request was routed to.

Assumptions/Requirements

This implementation assumes that caller will pass a ServerWebExchange as the Request Context.

It is also dependent on ServiceInstances having valid IDs (the field is nullable, so it might not be available, right?)

Rough spots

I wasn't seeing any existing context usage, so I just took a stab at it. Casting from an Object feels a little gross

Future work

Make the cookie name configurable
Make the way the cookie value is customized (people may want to encrypt/hash/do something to not expose server IDs)

[m11r9000]

Nice and tidy PoC @fitzoh . It was easy to follow 👍 . Some thoughts:

I can imagine that in some architectures other consumers will not be aware of other service instances, only the service id. I'm saying this as service instances are a bit ephemeral and consumers shouldn't need to know what service instance they should point to. Their only outlet should be the load balancer.

The strategy for checking what service instance to "stick to" should maybe be configurable so that any use-case can be plugged in? Some developers maybe have user_id's or affinity_key in the incoming requests and will have to perform HTTP requests towards every service instance to check if they own that specific id or key.

Regarding the usage of context in this case, I think one needs to make changes to the org.springframework.cloud.client.loadbalancer.reactive.ReactorLoadBalancerExchangeFilterFunction which intercepts the incoming request. In there, you would have to extract the appropriate cookie/header/whatever from the incoming ClientRequest and put it inside the Contextobject. One way would be doing it like this:

        Request<?> lbRequest = (request) -> request.attribute("affinity_key")
                .map((Function<Object, DefaultRequest<?>>) DefaultRequest::new)
                .orElse(new DefaultRequest<>());

So if there is an attribute value mapped to key affinity_key it will put that value in Context and create a new Request<> with that context.

And then the ReactorLoadBalancerExchangeFilterFunction would have to first fetch the appropriate load balancer for the specified serviceId:

ReactiveLoadBalancer<ServiceInstance> loadBalancer = loadBalancerFactory.getInstance(serviceId);

and finally shoot your new lbRequest like this:

loadBalancer.choose(lbRequest)

which will invoke StickySessionLoadBalancer#choose(Request)

If you want I could make some changes to your PoC, but would like to hear your opinions on this first :)

[fitzoh]

Nice and tidy PoC @fitzoh . It was easy to follow 👍

Thanks @andaziar!

I can imagine that in some architectures other consumers will not be aware of other service instances, only the service id. I'm saying this as service instances are a bit ephemeral and consumers shouldn't need to know what service instance they should point to. Their only outlet should be the load balancer.
The strategy for checking what service instance to "stick to" should maybe be configurable so that any use-case can be plugged in? Some developers maybe have user_id's or affinity_key in the incoming requests and will have to perform HTTP requests towards every service instance to check if they own that specific id or key.

I'm struggling a little bit to follow this... For reference, I'm not super familiar with the library and haven't used it directly (I'm primarily here because I saw spring-cloud/spring-cloud-gateway#1176 (comment) and have a use case for sticky sessions in spring-cloud-gateway).

Regarding the usage of context in this case, I think one needs to make changes to the org.springframework.cloud.client.loadbalancer.reactive.ReactorLoadBalancerExchangeFilterFunction which intercepts the incoming request. In there, you would have to extract the appropriate cookie/header/whatever from the incoming ClientRequest and put it inside the Context object.

One caveat here... It wouldn't be sufficient to just grab the value out of the cookie/header, we also need to be able to propagate the selected header back to the user.

If you want I could make some changes to your PoC, but would like to hear your opinions on this first :)

No objections here, but I'm definitely interested to hear what @OlgaMaciaszek has to say

[OlgaMaciaszek]

Hi @fitzoh @andaziar, sorry for getting back to you with a delay.
Thanks for the first draft @fitzoh . It's a good start, however, I agree with some remarks pointed out by @andaziar:

• I think we should not assume that the client already knows the instance id/ key; so if they do, we should take it from the cookies, but if not, we should pick an instance and propagate the id to the cookies
• This will, indeed, require modifying ReactorLoadBalancerExchangeFilterFunction to pass this value both ways, however, rather than just passing along that cookie value, I think it makes sense to start propagating the entire ClientRequest and ClientResponse objects (this has also been suggested in Lb complete lifecycle #733 (comment))

Apart from this, I think we can handle the cookies using ClientRequest objects that get passed via the LoadBalancerExchangeFilterFunction rather than ServerWebExchange.

I have created the first draft of possible LoadBalancerExchangeFunction changes so that we can discuss it here: #769. Let me know what you think?

Also, I'm going back to finishing up this PR now: #733 - will include the changes with passing along the ClientRequest and ClientResponse there. After the final version of that is merged, you could resubmit your sticky session PR as well.

CC @spencergibb @andersenleo

[cah-andrew-fitzgerald]

I think we should not assume that the client already knows the instance id/ key; so if they do, we should take it from the cookies, but if not, we should pick an instance and propagate the id to the cookies

I believe that's what I'm currently doing:

		// Check if the exchange has a cookie that points to a valid ServiceInstance
		return serviceInstanceFromCookie(exchange, instances)
				// if it does, then route to that server
				.map(instance -> Mono.just((Response<ServiceInstance>) new DefaultResponse(instance)))
				// otherwise we'll let the delegate pick a server
				.orElseGet(() -> delegate.choose(request))
				// either way we should set/renew the cookie
				.doOnNext(response -> setCookie(exchange, response));

This will, indeed, require modifying ReactorLoadBalancerExchangeFilterFunction to pass this value both ways

👍

I have created the first draft of possible LoadBalancerExchangeFunction changes so that we can discuss it here: #769. Let me know what you think?

Cool, I'll take a look

[OlgaMaciaszek]

@fitzoh you can also keep an eye on #733 as the discussion about modifying which data is going to be passed along through requests and responses will probably take place there.

[varna909]

Hi @spencergibb , @OlgaMaciaszek

Could you please let me know if the sticky session implementation is complete?Is there any docs on the usage?

[OlgaMaciaszek]

Hi, @fitzoh Would you like to continue working on this? If yes, please merge the current master (that now passes the ClientRequest object to the LB via ReactorLoadBalancerExchangeFilterFunction, switch your implementation to using the requests and add some tests. If you don't have time to work on it now, please let me know - I will get it done.

[fitzoh]

Interested in picking it back up, not sure on the timeline.
if it was something you were going to prioritize I would say go for it.

[OlgaMaciaszek]

Ok @fitzoh, could you give me some kind of estimate? If it's up to a month, we could wait. If it's longer, I'd rather get it done earlier. Let me know.

[fitzoh]

Ok @fitzoh, could you give me some kind of estimate? If it's up to a month, we could wait. If it's longer, I'd rather get it done earlier. Let me know.

Sorry for the delay @OlgaMaciaszek , I'd so go ahead and implement it