xds: Envoy should support its built-in LB policies as extensions

[markdroth]

LB extensibility was originally described in #5598. The infrastructure for this has been implemented, but there are a few missing pieces. Here's the relevant history:

• A new field for LB policy extensibility was added to xDS in #7744.
• The extension mechanism was added to Envoy in #17400.
• The semantics of the xDS field were changed slightly in #18419.
• xDS extension protos were added for ROUND_ROBIN and RING_HASH in #19517 and #20423, but Envoy does not yet actually implement configuring those policies as extensions.

I think the remaining work here is:

• Add extension protos for all built-in LB policies.
• Implement actually configuring these policies via the extension protos.

Once this is done, we can consider whether we want to deprecate the existing enum for configuring built-in policies.

[daixiang0]

Compared with enum, I prefer LB policies.

[wbpcode]

Great. This can make the envoy lb policies to be easy extended.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[markdroth]

Looks like a config for the least_request policy is being added in #21210.

[markdroth]

It looks like Envoy currently requires that the common_lb_config field is not set if the new load_balancing_policy field is used:

https://cs.github.com/envoyproxy/envoy/blob/6c75133dac08e6db4988d398479dc02a5dd8f11c/source/common/upstream/upstream_impl.cc#L990

I think this needs to be changed such that if the load_balancing_policy field is used, Envoy simply ignores the common_lb_config field. Otherwise, there's no way to cleanly switch from the old field to the new field as intended in https://github.com/envoyproxy/envoy/pull/18419.

[markdroth]

I think there's actually a more general problem here with the CommonLbConfig message, which is that it contains some fields that actually do apply only at the root of the LB policy tree. The example of this that we've run into is the override_host_status field, which is an override that applies at the top level of the LB policy tree only, since it essentially bypasses the normal LB policy completely.

We may want to consider moving override_host_status (and any other fields in CommonLbConfig that apply only to the top-level of the LB policy tree) out of CommonLbConfig and into the Cluster message itself. We should also consider moving the corresponding functionality out of the base LB policy class in Envoy, since in the case of nested LB policies, we would really want this functionality to be present only at the top level.

[markdroth]

I don't think this is done yet. We still need to actually implement the extensions, and figure out how to handle CommonLbConfig.

[wbpcode]

sorry, I should have closed it by mistake. We should still have a lot of work to do to complete this PR.

[wbpcode]

Seems like because of this. 🤣

[wbpcode]

Next we should make the subset lb as an extension.

[alyssawilk]

curious, is the plan here to port subset lb to be a thread aware load balancer, or extend TypedLoadBalancerFactory to be able to create non-thread-aware lbs? if the latter I'd be game for helping out!

[wbpcode]

@alyssawilk It would be great if you can help it. 😸

I think both of these are out targes. The first one is a mid-term goal. And the second one should be achieved partly for now. as we have created non-thread-aware lbs by the TypedLoadBalancerFactory. (Although I think there are some optimizations need to be landed.)

[alyssawilk]

have you? It looks like it only creates ThreadAwareLBs: https://github.com/envoyproxy/envoy/blob/main/envoy/upstream/load_balancer.h#L250 ?

I was thinking of extending it to be able to create non-thread-aware so we can move out subset but wasn't sure if you were already on it, or had different plans :-)

[wbpcode]

cc @markdroth I think this is basically done.

Although I will do some more optimization to it, like docs, examples, code optimizations, API frozen, etc.

[markdroth]

Awesome! Thank you so much for doing this!