RetryPriority plugin not supported for RING_HASH/MAGLEV
#9,042 opened on Nov 15, 2019
Repository metrics
- Stars
- (27,997 stars)
- PR merge metrics
- (Avg merge 8d) (378 merged PRs in 30d)
Description
RING_HASH and MAGLEV both support priority routing, but does not honor LoadBalancerContext::determinePriorityLoad, effectively breaking the RetryPredicate plugins. This also has implications wrt https://github.com/envoyproxy/envoy/pull/7967, as it attempts to target a specific priority using the LB context.
Calling determinePriorityLoad is tricky in the context of ThreadAwareLoadBalancer due to the LBs not having access to the PrioritySet used to compute the priority loads that it uses: only the necessary information is currently snapped into the TLS load balancer, which does not include the entire PrioritySet.
Some options:
- Pull the
PrioritySetfrom the TLS cluster. This is probably the simplest solution, but means that thePrioritySetmight be out of sync with the priority loads stored on theThreadAwareLoadBalancer: there might have been another change to the priority loads on the main thread before any given change is propagated to the TLS cluster. For example, if two updates happen right after each other, a request that gets handled between the two dispatcher events to update the TLS cluster might be using the TLS cluster after the first update but the priority loads after both updates. - Store additional data in the the LB factory, sufficient to recreate the
PrioritySeton the worker threads and pass it to the LB context. ThePrioritySetImplitself cannot be shared between threads due to being mutable. - Add an overload to
determinePriorityLoadthat doesn't accept aPrioritySet. This wouldn't make the ThreadAware LBs compatible with theRetryPriorityplugins, but would at least allow for the aggregate cluster to target a specific priority.
Open to other suggestions as well.
@yxue