xds: implement least_request load balancing policy #8739
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implements least_request as defined by [A48](https://github.com/grpc/proposal/blob/master/A48-xds-least-request-lb-policy.md)
erikjoh
commented
Dec 3, 2021
Forgot to make this package private from the start
These tests are mostly just a copy of RoundRobinLoadBalancerTest. The main difference is currently in the pickerLeastRequest test case. All other tests should be the same.
ejona86
reviewed
Dec 22, 2021
xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancerProvider.java
Outdated
Show resolved
Hide resolved
xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancerProvider.java
Outdated
Show resolved
Hide resolved
| public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { | ||
| LeastRequestConfig config = | ||
| (LeastRequestConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); | ||
| // Config may be null if least_request is used outside xDS |
There was a problem hiding this comment.
Is that so? I guess it may be if it becomes the default LB? We may want to fix that in gRPC.
There was a problem hiding this comment.
When trying this LB implementation together with a custom dns-based resolver it ended up being null here.
I haven't dug into exactly why this is the case however the custom resolver never sets any service config in the resolution result and also loadBalancingPolicyConfig is marked with @Nullable here which also is a reason to do a null check.
…actory Needed since InternalLimitedInfoFactory was removed in: grpc#8768
This keeps behavior completely consistent with now when least request is disabled. That is, fail clients with an error that LEAST_REQUEST isn't supported. Also introduce a system property to enable. This allows enablement through Java code.
|
@ejona86 This is ready for another pass when you have time. |
ejona86
approved these changes
Jan 13, 2022
ejona86
added
the
kokoro:run
grpc-kokoro
removed
the
kokoro:run
xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancerProvider.java
Outdated
Show resolved
Hide resolved
This change contains a few various fixes: * Fixes bug in ReadyPicker equality implementation that prevented runtime updates to choiceCount in some cases * Adds additional test and testing logic to ensure the ReadyPicker is created with the correct args * Replaces FakeRandom with a normal mock instead * Move the inFlights increment into the streamCreated hook instead
dapengzhang0
approved these changes
Jan 19, 2022
dapengzhang0
approved these changes
Jan 19, 2022
|
Could someone help with triggering kokoro and merge (unless there aren't any other reviewers) |
dapengzhang0
added
the
kokoro:run
grpc-kokoro
removed
kokoro:run
|
Thanks, @erikjoh! That was more involved than I had hoped. Thank you for the prompt iterations. @dapengzhang0, there was talk of backporting this to 1.44 to make it easier to try out. It is disabled by default, so seems reasonably safe. You think it makes sense? |
|
I think it's reasonably safe to backport too. |
ejona86
pushed a commit
to ejona86/grpc-java
that referenced
this pull request
Jan 19, 2022
Implements least_request_experimental as defined by [A48](https://github.com/grpc/proposal/blob/master/A48-xds-least-request-lb-policy.md) These tests are mostly just a copy of RoundRobinLoadBalancerTest. The main difference is currently in the pickerLeastRequest test case. All other tests should be the same.
ejona86
pushed a commit
that referenced
this pull request
Jan 19, 2022
Implements least_request_experimental as defined by [A48](https://github.com/grpc/proposal/blob/master/A48-xds-least-request-lb-policy.md) These tests are mostly just a copy of RoundRobinLoadBalancerTest. The main difference is currently in the pickerLeastRequest test case. All other tests should be the same.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements least_request as defined by
A48