envoyproxy/envoy

RetryPriority plugin not supported for RING_HASH/MAGLEV

Open

#9,042 opened on Nov 15, 2019

View on GitHub
 (2 comments) (1 reaction) (0 assignees)C++ (5,373 forks)batch import
area/load balancingbughelp wantedtech debt

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:

  1. Pull the PrioritySet from the TLS cluster. This is probably the simplest solution, but means that the PrioritySet might be out of sync with the priority loads stored on the ThreadAwareLoadBalancer: 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.
  2. Store additional data in the the LB factory, sufficient to recreate the PrioritySet on the worker threads and pass it to the LB context. The PrioritySetImpl itself cannot be shared between threads due to being mutable.
  3. Add an overload to determinePriorityLoad that doesn't accept a PrioritySet. This wouldn't make the ThreadAware LBs compatible with the RetryPriority plugins, but would at least allow for the aggregate cluster to target a specific priority.

Open to other suggestions as well.

@yxue

Contributor guide