Repository navigation
grpc-xds/load_balancing: WRR Locality LB implementation - #2950
nathanielford wants to merge 16 commits into
Conversation
# Conflicts: # grpc-xds/Cargo.toml
# Conflicts: # grpc-xds/src/load_balancing/mod.rs
| if targets.is_empty() { | ||
| return self.fail_update( | ||
| format!("{POLICY_NAME}: no localities with positive weight in resolver update"), | ||
| channel_controller, | ||
| ); | ||
| } |
There was a problem hiding this comment.
All other implementations seem to send an empty config to the child LB instead of dropping it. This allows the child to either cache the children or drop them immediately. This also allows the child to set the picker. We can follow the same behaviour in rust.
…ature/wrr-locality-lb # Conflicts: # grpc-xds/Cargo.toml
# Conflicts: # grpc-xds/Cargo.toml # grpc/Cargo.toml # grpc/src/client/load_balancing/mod.rs
|
@arjan-bal I think everything has been addressed - let me know if you have any other concerns! |
| // Update with an endpoint missing locality weight forwards empty targets | ||
| // to the child policy rather than dropping the child. |
There was a problem hiding this comment.
This is true for the mock balancer being used here. Since weighted_target is merged, can we use the read weighted_target and verify that a failing picker is produced?
Can you also see if other tests can use the real weighted_target LB now?
| impl WrrLocalityPolicy { | ||
| // Drops any active child policy, transitions the channel to | ||
| // TRANSIENT_FAILURE with a FailingPicker, and returns the error. | ||
| fn fail_update( |
There was a problem hiding this comment.
In the two callers, can you also include the resolution node from the resolver update so it's not lost?
| // Formats the locality as `{region="<region>", zone="<zone>", | ||
| // sub_zone="<sub_zone>"}`: the name of the locality's child policy under | ||
| // `xds_wrr_locality`, and its metric label value (gRFC A78). |
There was a problem hiding this comment.
This should be a /// comment.
|
I think everything is addressed - fixed the comment, carried through the note. I'm using the |
Motivation
Implementation of xDS WRR Locality LB (#2763).
Solution
Leverages
Child(fromChildManager, though it does not use the manager itself) to manage theweighted_target_experimentalchild policy. Carries functionality to parsexds_wrr_locality_experimentalconfiguration, extract locality weights from endpointResolutionResultattributes, dynamically build theweighted_target_experimentalchild configuration per locality, and delegate updates and state management to the child policy.Notes
weighted_targetLB policy. Mocks are used in cases where checking the weights is necessary, or scripting child behavior, but weighted target is used when we want to ensure the integrated behavior.