-
Notifications
You must be signed in to change notification settings - Fork 855
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
Request can be affinitized to destinations #174
Request can be affinitized to destinations #174
Conversation
…ed from cookie or custom header
PR is created to get an early feedback on the approach and not completed yet. The following is still in progress:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please file an issue for the cross-destination affinity provider we discussed.
src/ReverseProxy/Abstractions/BackendDiscovery/Contract/SessionAffinityMode.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/ISessionAffinityFeature.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/ISessionAffinityFeature.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/SessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/SessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/ISessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/ISessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/SessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/AffinitizationResult.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/ISessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
I noticed an issue with setting the affinity cookie before calling HttpProxy. HttpProxy copies back response headers using TryAdd
This means if we add a Set-Cookie response header for affinity then any Set-Cookie response headers from the destination won't be sent back to the client. I think HttpProxy is using the wrong API regardless. It should be using Set/Indexer or Append. Set would make sense for replacing things like the default server header. Append would make sense for our cookie scenario. Try changing it to Append for now and we'll see what happens. |
src/ReverseProxy/Service/SessionAffinity/SessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
…dleware types - Extensible name-based affinity provider configuration - Affinity cookie properties can be customized - HttpProxy calls Append to set downstream response headers - Affinity key is set on a downstream response right after request gets affinitized
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making good progress. I didn't review everything, but I wanted to send you the feedback I have so far. I'll be off tomorrow.
src/ReverseProxy/Abstractions/BackendDiscovery/Contract/CookieSessionAffinityProviderOptions.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Abstractions/BackendDiscovery/Contract/CookieSessionAffinityProviderOptions.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Abstractions/BackendDiscovery/Contract/SessionAffinityOptions.cs
Show resolved
Hide resolved
...eProxy/Configuration/DependencyInjection/BuilderExtensions/IReverseProxyBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Middleware/AffinitizedDestinationLookupMiddleware.cs
Outdated
Show resolved
Hide resolved
- AffinitizeRequestMiddleware picks a random destination if there are multiple of them - Affinity key is not logged anymore - IOption<TOption> is used for CookieAffinitySessionProvider
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failure modes look like the next section to work on?
src/ReverseProxy/Middleware/ProxyMiddlewareAppBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Middleware/ProxyMiddlewareAppBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Middleware/ProxyMiddlewareAppBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/ISessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/CookieSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
- Affinity key data protection - Session affinity configuration validation - Fixed comments - Extra logging
@Tratcher I implemented handling of missing affinitized destinations with PickRandom and ReturnError strategies. That failure type looks like the only one we want to currently handle in session affinity logic. |
...eProxy/Configuration/DependencyInjection/BuilderExtensions/IReverseProxyBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
...eProxy/Configuration/DependencyInjection/BuilderExtensions/IReverseProxyBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Middleware/AffinitizedDestinationLookupMiddleware.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Middleware/AffinitizedDestinationLookupMiddleware.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
- AffinitizeRequestMiddleware recreates the destination list only if there are still multiple destinations available - AddDataProtection method removed from proxy's API. DataProtection is set up in the ReverseProxy.Sample
- BaseSessionAffinityProviderTest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incremental feedback, mostly style and flow control concerns.
src/ReverseProxy/Abstractions/BackendDiscovery/Contract/SessionAffinityBuiltIns.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Middleware/AffinitizedDestinationLookupMiddleware.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Middleware/AffinitizedDestinationLookupMiddleware.cs
Outdated
Show resolved
Hide resolved
…nto alnikola/45-session-affinity
- AffinitizeRequest refactored - Other small fixes to address comments
src/ReverseProxy/Middleware/AffinitizedDestinationLookupMiddleware.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/BaseSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
- Naming fix
- AffinityDisabled status is removed
It needs to be yet decided whether we really need a destination lookup table in the default implementation. |
PR is ready for review.There is still one open question above. However, even if we decide to implement it by default, not sure if it should be part of this PR since it already quite big. |
It was decided to separately implement a destination lookup table as a concrete application of a generic component-specific data storage discussed in #229. |
src/ReverseProxy/Abstractions/BackendDiscovery/Contract/SessionAffinityOptions.cs
Show resolved
Hide resolved
src/ReverseProxy/Abstractions/BackendDiscovery/Contract/SessionAffinityDefaultOptions.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Middleware/AffinitizedDestinationLookupMiddleware.cs
Outdated
Show resolved
Hide resolved
This is getting much more polished. Good job. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Polish
src/ReverseProxy/Service/SessionAffinity/CustomHeaderSessionAffinityProvider.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/CustomHeaderSessionAffinityProvider.cs
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/IAffinityFailurePolicy.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/IAffinityFailurePolicy.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/Return503ErrorAffinityFailurePolicy.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Service/SessionAffinity/SessionAffinityMiddlewareHelper.cs
Outdated
Show resolved
Hide resolved
src/ReverseProxy/Middleware/AffinitizedDestinationLookupMiddleware.cs
Outdated
Show resolved
Hide resolved
var keepProcessing = await _operationLogger.ExecuteAsync("ReverseProxy.HandleAffinityFailure", async () => | ||
{ | ||
var failurePolicy = _affinityFailurePolicies.GetRequiredServiceById(options.AffinityFailurePolicy); | ||
return await failurePolicy.Handle(context, options, affinityResult.Status); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design: What if the failure handler needs to remove a some affinity relationship state? E.g. if the affinity is tracked in some other table? It doesn't have access to the provider's implementation details. Would we add a cleanup method to the provider interface and then pass that provider in here?
Or do we always rely on AffinitizeRequest to overwrite any old state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it might be needed in the future to implement cross-backend affinity failure handling, but I believe we shouldn't mix it in this PR.
Regarding the contract, I'd prefer to pass a more specialized interface (e.g. IAffinityStateCleaner
) not the full ISeesionAffinityProvder
to the failure policy to avoid too strong coupling.
- DynamicConfigBuilder directly reads the session affinity constants - SessionAffinityOptions.Enabled added - AffinityFailurePolicies throw exceptions if invoked for a successful status - Wording fixed - More debug logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
Next can do a PR for a one page doc on how to use it?
- What it's for
- How to turn it on in config
- Which modes are available and how they work
- What happens when affinity fails
No need to cover custom providers yet.
src/ReverseProxy/Middleware/AffinitizedDestinationLookupMiddleware.cs
Outdated
Show resolved
Hide resolved
Document explaining the design of Session Affinity feature implemented in #174
Document explaining the design of Session Affinity feature implemented in #174
Request can be affinitized to destinations by an affinity key extracted from cookie or custom header. Affinity is active only when load balancing is enabled and it can be configured for each backend independently.
One request can have affinity to a single destination or to a set. It enables to scenarios where a session needs to be affinitized to a pool of load-balanced destinations.
Fixes #45
Session Affinity
Concept
Session affinity is a mechanism to bind (affinitize) a causally related request sequence to the destination handled the first request when the load is balanced among several destinations. It is useful in scenarios where the most requests in a sequence work with the same data and the cost of data access differs for different nodes (destinations) handling requests. The most common example is a transient caching (e.g. in-memory) where the first request fetches data from a slower persistent storage into a fast local cache and the others work only with the cached data thus increasing throughput.
Affinity Key
Request to destination affinity is established via the affinity key identifying the target destination. That key can be stored on different request parts depending on the given session affinity implementation, but each request cannot have more than one such key.
The current design doesn't require for key to uniquely identify the single affinitized destination, it's allowed to establish affinity to a destination group. In such case, the exact destination to handle the given request will be determined by the load balancer.
Establishing a new affinity or resolution of the existed one
Once a request arrives and gets routed to a backend with enabled session affinity, the proxy automatically decides whether a new affinity should be established or an existing one needs to be resolved based on the presence and validity of an affinity key on the request as follows:
If a new affinity was established for the request, the affinity key gets attached to a response where exact key representation and location depends on the implementation. Currently, there are two built-in providers storing the key on a cookie or custom header. Once the response gets delivered to the downstream client, it's the client responsibility to attach the key to all following requests in the same session. Further, when the next request carrying the key arrives to the proxy, it resolves the existing affinity, but affinity key does not get again attached to the response. Thus, only the first response carries the affinity key.
Affinity failure policy
If the affinity key cannot be decoded or no target destination found, it's considered as a failure, and an affinity failure policy is called to handle it. The policy has the full access to
HttpContext
and can send response to the client by itself. It returns a boolean value indicating whether the request processing can proceed further or must be terminated.Configuration
Session affinity implemented by services and middleware. Services are added via
AddSessionAffinityProvider()
. Middleware is added viaUseAffinitizedDestinationLookup()
andUseRequestAffinitizer()
. Note, the first method must be called before addingLoadBalancingMiddleware
whereas the second must be called after.Further, session affinity is configured per backend as in the example below: