proposal
proposal copied to clipboard
grpc
A55: Stateful session affinity for gRPC clients
On Fri, Sep 9, 2022 at 4:56 PM Israel Shapiro @.***> wrote:
@.**** commented on this pull request.
In A55-stateful-session-affinity.md https://github.com/grpc/proposal/pull/319#discussion_r967545404:
+total number of backends) which would lead to dropped requests in some cases +which is unacceptable.
+This feature avoids the problem by using HTTP cookies as described above. + +## Implementation + +The design will be implemented in gRPC C++, Java and Go - in that order. +Wrapped languages should be able to use the feature provided they can do +cookie management in their client code. + +Cookies are rarely used with gRPC so the implementations should also +include a recipe for cookie management. This could involve illustrative +examples using cookie jar implementations which are available in various +languages such as Java, Go and Node.js. Wherever possible the +implementations may also include a reference implementation of an interceptor
The intermediate nodes could create their own cookies and add them to the RPC responses. As long as the client application copies those cookies to the following requests you have session affinity through the entire path, no?
I think that should work.
Yes - AFAIK it works today too, if the user does it in their own code. The question is: if user enables 'persistent session', and the framework does it automatically for the direct-connected backend case - wouldn't be reasonable to also handle this case ? Because if the user has to do the work for 1/2 of the cases - it would be much simpler for the LB to not be involved at all in the cookie management, and just allow the user to indicate the preferred backend.
If the client LB is the only piece making the decision to keep requests going to same IP - why all this complexity with a cookie and all the other layers ? As I mentioned, client-side stickiness is currently possible from client code ( Dial(IP) - if it fails Dial(service) and check peer IP for future requests). Persistent sessions using server-generated cookie and using server infra is also possible - clients can check MD for cookie and pass it. There is no change needed for either of this.
As I mentioned earlier, ideally the server could generate a cookie that identifies the server state for a set of RPC requests. If we had a LB infra that could be configured to properly guarantee session affinity based of the server-side generated cookie that would be sufficient. But that's not the case. None of currently supported LB or routing modes in Envoy implement proper affinity (only soft affinity) based on an HTTP header or cookie.
Sure, but Envoy is not the only server in the world - and if it has limitations it doesn't mean they can't be fixed. We do have plenty of LB infra that can be configured to generate cookies and do proper affinity. Even with Envoy it is possible to have the server code generate a cookie - in the rudimentary 'base64(IP)' form - and it would work in the same limited case.
I don't think it should be the role of the client to have the logic to dial to specific endpoint IPs to achieve that affinity, including in cases where they are connecting through E-W gateways. In some topologies the clients are connecting to a VIP and would be seeing a single IP overall. When xDS comes into the picture the client has no knowledge or control of the endpoint being selected for each and every request.
Don't we still have a method to return the peer IP ? And Dial(IP) without XDS prefix is bypassing xDS. I'm pretty sure I used this pattern at some point and it worked.
I also don't think it should be the 'role of the client' - yet a lot of clients have smart sharding or other custom discovery logic (bypassing K8S or XDS) and do this from time to time, and with a small helper around it's not that hard. It's a huge pain for Istio - all the stateful sets and 'custom discovery' were a nightmare to support properly and we still have some gaps, but in most cases clients had pretty good reasons to do this.
I agree that it would be convenient for client to be able to pass the IP to the LB - like in Envoy, where a header or cookie can be used - and avoid some of the complexity.
The client doesn't pass an IP to the LB, it treats the cookie as an opaque object.
Well - an opaque object that must be base64(IP), and LB expect it to be exactly this.
So instead of a simple and clean Dial(IP) - or LB remembering the IP and storing it in a field - we base64 encode, add a cookie jar, do path and host maching, base64 decode - and get back the same IP we had in the first place. While adding another dependency to a cookie jar library, because code size is not yet bloated enough Sorry if I'm a bit sarcastic - the same pattern is quite frequent in Istio and in a lot of other places too :-)
But calling it 'client generated cookie' would be extremely confusing for anyone who used cookies, and supporting only this limited case - and not supporting the wast number of load balancers supporting 'normal' session persistent - is not ideal.
The cookie being discussed here is not 'client generated'. it is generated by the LB infra that happens to be part of the client gRPC core layer.
If you want to call a library or framework used by client 'a server' having to exchange data with the client using a cookie jar - maybe :-)
At the minimum the client should store and pass back the cookie - if it can't use it directly - so all this infra can work.
Thats exactly what is being assumed here.
I'm not entirely sure anymore... So the LB library returns the same IP that Peer IP would return - but as a cookie, in base 64. And the client pass it on the next call using a cookie jar, the LB decodes the cookie, verifies the current connection IP is likely the same ( and if disconnected
- redials the same IP it was just connected to, but from the cookie value ) ? And if LB can't decode the cookie, just pass it through since it's likely to be a proper server-generated session cookie for the LBs to handle ?
It might be possible to do this using separate cookies - and letting user code deal with persistent sessions when intermediary LBs are in the path ( AFAIK this should already work ). So maybe all we need is to just check if server returns a cookie and use that instead ( assuming some LB or server is managing the persistence), otherwise just keep track the IP address.
To make this work, there would need to be a [server-generated-cookie->serverIP] mapping state that needs to be maintained internally as part of the LB layer in gRPC and/or Envoy. This is not how it is currently implemented in Envoy.
AFAIK - that's close to how it's implemented in Envoy, they also support the header that contains the same server IP ( based on 'original dst' ). And a client can take any IP, base64 or set it as a header, call Envoy - and AFAIK it will connect to that IP if the settings are right.
That's how I'm planning to implement stateful sets and persistent sessions for the multi-network/E-W gateway - so I hope it'll work...
On Mon, Sep 12, 2022 at 9:19 AM sanjaypujare @.***> wrote:
@.**** commented on this pull request.
In A55-stateful-session-affinity.md https://github.com/grpc/proposal/pull/319#discussion_r968616008:
@@ -0,0 +1,348 @@ +A55: Stateful Session Affinity for Proxyless gRPC
And I think it should be possible to configure the HTTP filter via the gRPC service config without too much trouble.
Curious if we have examples of a feature using an http-filter that's configured through service config (and not xds?)
(long term) Wouldn't it be nice to allow ALL features that are configured via XDS to also be configurable locally ? Or at least the features that are implemented as filters/interceptors - it is not impossible to expose them as 'general purpose' with a local config, and have more parity and easier migration.
Message ID: @.***>
(to avoid confusion - my previous comment is for 'long term' - short term we need to use what is currently available )
On Fri, Sep 16, 2022 at 7:52 AM Costin Manolache @.***> wrote:
Header is easier and probably better for most use cases in the mesh.
Cookie's main use case is browsers - without any special JS code. For programmatic/JS requests - header is better even in browsers.
I think it is very important to support cookies for integration with existing infrastructure and use outside of the mesh - for example most proxies (nginx, etc) and many managed services (like CloudRun) will use cookies. However - this already works and doesn't need changes in gRPC - the request is sent to the gateway, no special LB decision needed.
Please also consider Istio with the new Ambient model - gRPC and HTTP requests will be routed by client to the Waypoint server, which will be responsible to handle the persistence. The model where gRPC LB tracks the upstream backend and keeps using it will not work - the header or cookie needs to be generated by the Waypoint server or backend itself ( client -> Waypointserver -> sticky backend ).
On Fri, Sep 16, 2022 at 7:07 AM Mark D. Roth @.***> wrote:
@.**** commented on this pull request.
In A55-stateful-session-affinity.md https://github.com/grpc/proposal/pull/319#discussion_r973064651:
+total number of backends) which would lead to dropped requests in some cases +which is unacceptable.
+This feature avoids the problem by using HTTP cookies as described above. + +## Implementation + +The design will be implemented in gRPC C++, Java and Go - in that order. +Wrapped languages should be able to use the feature provided they can do +cookie management in their client code. + +Cookies are rarely used with gRPC so the implementations should also +include a recipe for cookie management. This could involve illustrative +examples using cookie jar implementations which are available in various +languages such as Java, Go and Node.js. Wherever possible the +implementations may also include a reference implementation of an interceptor
BTW, it looks like we aren't the only ones talking about possibly using a header instead of a cookie here. I just noticed envoyproxy/envoy#23145 https://github.com/envoyproxy/envoy/pull/23145, which is proposing adding header-based support for stateful session affinity.
— Reply to this email directly, view it on GitHub https://github.com/grpc/proposal/pull/319#discussion_r973064651, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUR2TWKKJ4RNIIFH7P3QTV6R5IHANCNFSM56XON7TQ . You are receiving this because you were mentioned.Message ID: @.***>
On Tue, Sep 13, 2022 at 1:28 PM Mark D. Roth @.***> wrote:
@.**** commented on this pull request.
In A55-stateful-session-affinity.md https://github.com/grpc/proposal/pull/319#discussion_r970054410:
+total number of backends) which would lead to dropped requests in some cases +which is unacceptable.
+This feature avoids the problem by using HTTP cookies as described above. + +## Implementation + +The design will be implemented in gRPC C++, Java and Go - in that order. +Wrapped languages should be able to use the feature provided they can do +cookie management in their client code. + +Cookies are rarely used with gRPC so the implementations should also +include a recipe for cookie management. This could involve illustrative +examples using cookie jar implementations which are available in various +languages such as Java, Go and Node.js. Wherever possible the +implementations may also include a reference implementation of an interceptor
@costinm https://github.com/costinm It is definitely not feasible for clients to create a new channel directly to the IP address that they want the session to go to. That approach would lose all sorts of important xDS-configured things (e.g., mTLS config, retry config, fault injection, etc), and it would lose the ability to fall back to a different endpoint in the event that the original one actually does go down.
That having been said, I do think that there is a general problem here, as pointed out by @ejona86 https://github.com/ejona86 offline, which is that by using a different cookie jar for each session, the client application is making assumptions about what the xDS config is, which means that the xDS config cannot change things in ways that are incompatible with what the client is doing. I think the example cited above (the traffic going through an intermediate proxy that may set its own cookie, which the client might wind up applying to just one session) is just one example of how this can be a problem.
I think we should consider whether there are ways to ameliorate this problem. For example, instead of using cookies, which already have existing semantics that do not match what would need to happen here, what if we just used a configurable header? That would still require explicit cooperation from the application, but the mechanism would not make any assumptions about existing semantics (e.g., cookies that are not intended to be tied to one particular session), so it may be less likely to break things. The down-side is that it would be more work for the application to do this, because it could not just use an off-the-shelf cookie jar. But the trade-off in flexibility may be worth that additional work.
Thoughts...?
As I mentioned in a different thread, cookies are important for interop with other servers - that's how persistent sessions are implemented by a lot of infra and services - and AFAIK they mostly work today, the persistence is handled by the infra LB not by client, and gRPC has access to headers (MD), no changes should be needed.
Having a simpler/cleaner header-based solution is better in many ways, I think there is a PR in progress to use headers in envoy and if it merges it would be great to match it in gRPC.
Regarding 'direct connection to IP' - I agree, it is not ideal and has a lot of problems, with Envoy you also lose TLS verification and most policies. Yet it is quite commonly done by some apps (not sure how common with gRPC vs HTTP or TCP protocols, but I've seen code using Dial(IP) for this purpose) - user code is complex, sometimes they implement their own TLS and handle failures, discovery and connecting to other hosts etc. A lot of apps need persistent sessions - and in the absence of a supported solution they do what they have to do. That's why it would be great to have this persistent session good enough to replace the need of users to implement their own on top of Dial(IP). We have a good start - but there are still a lot of limitations and problems, in particular if other Istio features are in use ( gateways, middle boxes, the new ambient, subset support, traffic shifting etc).
Message ID: @.***>
After discussing some more of the fine details here with some of the Envoy folks, I think we need to take a small step back and define how we want stateful session affinity to work for DRAINING endpoints from a xDS perspective -- i.e., we should define what the desired behavior is and then make sure that both Envoy and gRPC provide that behavior.
From reading the Envoy code, it looks to me like Envoy will actually reconnect to the DRAINING endpoint if that endpoint is picked by the override-host logic. I don't see anything in the connection-pool code that avoids creating a new connection if the host is unhealthy, so I'm guessing that that normally relies on the LB policy to avoid picking an unhealthy host -- and of course, that behavior is bypassed for the override-host logic based on the override_host_status set. So if the override-host logic picks the host and asks the connection pool for a connection, it seems like it will create a new one, not caring that the endpoint is in state DRAINING. But I'd like confirmation from the Envoy folks that I'm reading this correctly.
Is that the behavior we want here? It certainly seems like the right behavior from the perspective of the actual use-case that is prompting this design, which involves delaying the full shutdown of the draining endpoints until the already-in-flight sessions are finished (which the application must have some way to know externally to xDS). But if we agree on this, we need the Envoy folks to agree not to break it -- i.e., this means that we cannot make a change in the future that would change Envoy's behavior to avoid reconnecting to hosts in state DRAINING.
If we agree that this is the right behavior, we can implement this in gRPC by having the override_host_experimental LB policy actively maintain a connection to a DRAINING endpoint for as long as it exists in the EDS resource (i.e., we would try to reconnect if the existing connection breaks).
@htuch @yanavlasov @wbpcode for input from the Envoy side.
Is that the behavior we want here? It certainly seems like the right behavior from the perspective of the actual use-case that is prompting this design, which involves delaying the full shutdown of the draining endpoints until the already-in-flight sessions are finished (which the application must have some way to know externally to xDS).
Yeah. Although only healthy (include unknow and degraded) host will be selected by the override logic by default. But in some corner case, the users may want to keep the session as far as possible. This could be controlled by the override_host_status.
But if we agree on this, we need the Envoy folks to agree not to break it -- i.e., this means that we cannot make a change in the future that would change Envoy's behavior to avoid reconnecting to hosts in state DRAINING.
Because the panic mode will try to create connection to the unhealthy host also (as long as it exists in the host set). I think we won't break this in the future. (I will read related code to ensure again. -- new connection could be created for the unhealthy host and this behavior is what we want.)
Even if we want to change it someday (again, I think this is almost impossible to happen), a new proto API will be used to control this behavior.
Header is easier and probably better for most use cases in the mesh.
Cookie's main use case is browsers - without any special JS code. For programmatic/JS requests - header is better even in browsers.
I think it is very important to support cookies for integration with existing infrastructure and use outside of the mesh - for example most proxies (nginx, etc) and many managed services (like CloudRun) will use cookies. However - this already works and doesn't need changes in gRPC - the request is sent to the gateway, no special LB decision needed.
Please also consider Istio with the new Ambient model - gRPC and HTTP requests will be routed by client to the Waypoint server, which will be responsible to handle the persistence. The model where gRPC LB tracks the upstream backend and keeps using it will not work - the header or cookie needs to be generated by the Waypoint server or backend itself ( client -> Waypointserver -> sticky backend ).
On Fri, Sep 16, 2022 at 7:07 AM Mark D. Roth @.***> wrote:
@.**** commented on this pull request.
In A55-stateful-session-affinity.md https://github.com/grpc/proposal/pull/319#discussion_r973064651:
+total number of backends) which would lead to dropped requests in some cases +which is unacceptable.
+This feature avoids the problem by using HTTP cookies as described above. + +## Implementation + +The design will be implemented in gRPC C++, Java and Go - in that order. +Wrapped languages should be able to use the feature provided they can do +cookie management in their client code. + +Cookies are rarely used with gRPC so the implementations should also +include a recipe for cookie management. This could involve illustrative +examples using cookie jar implementations which are available in various +languages such as Java, Go and Node.js. Wherever possible the +implementations may also include a reference implementation of an interceptor
BTW, it looks like we aren't the only ones talking about possibly using a header instead of a cookie here. I just noticed envoyproxy/envoy#23145 https://github.com/envoyproxy/envoy/pull/23145, which is proposing adding header-based support for stateful session affinity.
— Reply to this email directly, view it on GitHub https://github.com/grpc/proposal/pull/319#discussion_r973064651, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUR2TWKKJ4RNIIFH7P3QTV6R5IHANCNFSM56XON7TQ . You are receiving this because you were mentioned.Message ID: @.***>
Header is easier and probably better for most use cases in the mesh. Cookie's main use case is browsers - without any special JS code. For programmatic/JS requests - header is better even in browsers. I think it is very important to support cookies for integration with existing infrastructure and use outside of the mesh - for example most proxies (nginx, etc) and many managed services (like CloudRun) will use cookies. However - this already works and doesn't need changes in gRPC - the request is sent to the gateway, no special LB decision needed. Please also consider Istio with the new Ambient model - gRPC and HTTP requests will be routed by client to the Waypoint server, which will be responsible to handle the persistence. The model where gRPC LB tracks the upstream backend and keeps using it will not work - the header or cookie needs to be generated by the Waypoint server or backend itself ( client -> Waypointserver -> sticky backend ). … On Fri, Sep 16, 2022 at 7:07 AM Mark D. Roth @.> wrote: @.* commented on this pull request. ------------------------------ In A55-stateful-session-affinity.md <#319 (comment)>: > +total number of backends) which would lead to dropped requests in some cases +which is unacceptable. + +This feature avoids the problem by using HTTP cookies as described above. + +## Implementation + +The design will be implemented in gRPC C++, Java and Go - in that order. +Wrapped languages should be able to use the feature provided they can do +cookie management in their client code. + +Cookies are rarely used with gRPC so the implementations should also +include a recipe for cookie management. This could involve illustrative +examples using cookie jar implementations which are available in various +languages such as Java, Go and Node.js. Wherever possible the +implementations may also include a reference implementation of an interceptor BTW, it looks like we aren't the only ones talking about possibly using a header instead of a cookie here. I just noticed envoyproxy/envoy#23145 <envoyproxy/envoy#23145>, which is proposing adding header-based support for stateful session affinity. — Reply to this email directly, view it on GitHub <#319 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUR2TWKKJ4RNIIFH7P3QTV6R5IHANCNFSM56XON7TQ . You are receiving this because you were mentioned.Message ID: @.***>
It would be pretty easy to add header support if this proposal is done finally. Just like it's very easy to add header based stateful session in the envoy.
My real point in this thread is that it would be great to simplify this where possible.
We must use a cookie jar for integrating with other infra that uses cookies for persistence - but in the simpler case of direct communication it should be possible to have less overhead.
And I expect Envoy to improve as well - using External Authz it is already partially possible to implement something equivalent ( ext authz can modify the headers and likely can insert this cookie). I suspect this feature will be pretty popular and users will ask for the other features, and the hard part is already in place.
On Fri, Sep 9, 2022 at 8:10 PM Costin Manolache @.***> wrote:
On Fri, Sep 9, 2022 at 4:56 PM Israel Shapiro @.***> wrote:
@.**** commented on this pull request.
In A55-stateful-session-affinity.md https://github.com/grpc/proposal/pull/319#discussion_r967545404:
+total number of backends) which would lead to dropped requests in some cases +which is unacceptable.
+This feature avoids the problem by using HTTP cookies as described above. + +## Implementation + +The design will be implemented in gRPC C++, Java and Go - in that order. +Wrapped languages should be able to use the feature provided they can do +cookie management in their client code. + +Cookies are rarely used with gRPC so the implementations should also +include a recipe for cookie management. This could involve illustrative +examples using cookie jar implementations which are available in various +languages such as Java, Go and Node.js. Wherever possible the +implementations may also include a reference implementation of an interceptor
The intermediate nodes could create their own cookies and add them to the RPC responses. As long as the client application copies those cookies to the following requests you have session affinity through the entire path, no?
I think that should work.
Yes - AFAIK it works today too, if the user does it in their own code. The question is: if user enables 'persistent session', and the framework does it automatically for the direct-connected backend case - wouldn't be reasonable to also handle this case ? Because if the user has to do the work for 1/2 of the cases - it would be much simpler for the LB to not be involved at all in the cookie management, and just allow the user to indicate the preferred backend.
If the client LB is the only piece making the decision to keep requests going to same IP - why all this complexity with a cookie and all the other layers ? As I mentioned, client-side stickiness is currently possible from client code ( Dial(IP) - if it fails Dial(service) and check peer IP for future requests). Persistent sessions using server-generated cookie and using server infra is also possible - clients can check MD for cookie and pass it. There is no change needed for either of this.
As I mentioned earlier, ideally the server could generate a cookie that identifies the server state for a set of RPC requests. If we had a LB infra that could be configured to properly guarantee session affinity based of the server-side generated cookie that would be sufficient. But that's not the case. None of currently supported LB or routing modes in Envoy implement proper affinity (only soft affinity) based on an HTTP header or cookie.
Sure, but Envoy is not the only server in the world - and if it has limitations it doesn't mean they can't be fixed. We do have plenty of LB infra that can be configured to generate cookies and do proper affinity. Even with Envoy it is possible to have the server code generate a cookie - in the rudimentary 'base64(IP)' form - and it would work in the same limited case.
I don't think it should be the role of the client to have the logic to dial to specific endpoint IPs to achieve that affinity, including in cases where they are connecting through E-W gateways. In some topologies the clients are connecting to a VIP and would be seeing a single IP overall. When xDS comes into the picture the client has no knowledge or control of the endpoint being selected for each and every request.
Don't we still have a method to return the peer IP ? And Dial(IP) without XDS prefix is bypassing xDS. I'm pretty sure I used this pattern at some point and it worked.
I also don't think it should be the 'role of the client' - yet a lot of clients have smart sharding or other custom discovery logic (bypassing K8S or XDS) and do this from time to time, and with a small helper around it's not that hard. It's a huge pain for Istio
- all the stateful sets and 'custom discovery' were a nightmare to support properly and we still have some gaps, but in most cases clients had pretty good reasons to do this.
I agree that it would be convenient for client to be able to pass the IP to the LB - like in Envoy, where a header or cookie can be used - and avoid some of the complexity.
The client doesn't pass an IP to the LB, it treats the cookie as an opaque object.
Well - an opaque object that must be base64(IP), and LB expect it to be exactly this.
So instead of a simple and clean Dial(IP) - or LB remembering the IP and storing it in a field - we base64 encode, add a cookie jar, do path and host maching, base64 decode - and get back the same IP we had in the first place. While adding another dependency to a cookie jar library, because code size is not yet bloated enough Sorry if I'm a bit sarcastic - the same pattern is quite frequent in Istio and in a lot of other places too :-)
But calling it 'client generated cookie' would be extremely confusing for anyone who used cookies, and supporting only this limited case - and not supporting the wast number of load balancers supporting 'normal' session persistent - is not ideal.
The cookie being discussed here is not 'client generated'. it is generated by the LB infra that happens to be part of the client gRPC core layer.
If you want to call a library or framework used by client 'a server' having to exchange data with the client using a cookie jar - maybe :-)
At the minimum the client should store and pass back the cookie - if it can't use it directly - so all this infra can work.
Thats exactly what is being assumed here.
I'm not entirely sure anymore... So the LB library returns the same IP that Peer IP would return - but as a cookie, in base 64. And the client pass it on the next call using a cookie jar, the LB decodes the cookie, verifies the current connection IP is likely the same ( and if disconnected
- redials the same IP it was just connected to, but from the cookie value ) ? And if LB can't decode the cookie, just pass it through since it's likely to be a proper server-generated session cookie for the LBs to handle ?
It might be possible to do this using separate cookies - and letting user code deal with persistent sessions when intermediary LBs are in the path ( AFAIK this should already work ). So maybe all we need is to just check if server returns a cookie and use that instead ( assuming some LB or server is managing the persistence), otherwise just keep track the IP address.
To make this work, there would need to be a [server-generated-cookie->serverIP] mapping state that needs to be maintained internally as part of the LB layer in gRPC and/or Envoy. This is not how it is currently implemented in Envoy.
AFAIK - that's close to how it's implemented in Envoy, they also support the header that contains the same server IP ( based on 'original dst' ). And a client can take any IP, base64 or set it as a header, call Envoy - and AFAIK it will connect to that IP if the settings are right.
That's how I'm planning to implement stateful sets and persistent sessions for the multi-network/E-W gateway - so I hope it'll work...
Just waiting for input from @ejona86 on the cookie order question. I've pinged him on this.
All comments addressed (AFAICS). @markdroth PTAL. Hopefully this can be approved now.
This looks great!
Thank you! Still need to wait for LGTMs from @ejona86 and @dfawley right? And after that wait for a week before merging? Just confirming
I don't think we need to wait for approvals from @ejona86 or @dfawley, since they were present in our internal design review. And the 1-week waiting period starts when the gRFC is announced, not when it's approved, so I don't think we need to wait any longer.
I'll go ahead and merge this now.