envoyproxy / envoyproxy/envoy

RetryPriority plugin not supported for RING_HASH/MAGLEV

Open
#9,042 2 comments 1 reaction 0 assignees View on GitHub
area/load balancing bug help wanted tech debt
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
428

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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.