core, grpclb: change policy selection strategy for Grpclb policy (take one: eliminate special logic for deciding grpclb policy in core) #6637
Conversation
This file contains hidden or 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
…olution result. Change AutoConfiguredLoadBalancerFactory to choose LB policy purely based on lb policy config in service config. GrpclbLoadBalancer populates balancer address from attributes instead of from addresses.
voidzcy force-pushed the impl/grpclb_selection_change_take_one branch from 4e33ec5 to c090767 Compare …election_change_take_one
voidzcy force-pushed the impl/grpclb_selection_change_take_one branch from c090767 to da29532 Compare voidzcy changed the title 
creamsoup reviewed Jan 28, 2020
core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java Show resolved Hide resolved
…f no usable addresses.
…n balancer addresses, this mirrors the real behavior of DnsNameResolver.
voidzcy force-pushed the impl/grpclb_selection_change_take_one branch from a9d6a40 to b428ae3 Compare voidzcy force-pushed the impl/grpclb_selection_change_take_one branch from b428ae3 to 5adefa9 Compare creamsoup approved these changes Jan 30, 2020
Contributor
creamsoup left a comment
creamsoup left a comment There was a problem hiding this comment.
LGTM, can you wait couple days. it would be nice to verify "service config error handling" without this change.
voidzcy force-pushed the impl/grpclb_selection_change_take_one branch 2 times, most recently from 8f38f42 to ab974ef Compare voidzcy force-pushed the impl/grpclb_selection_change_take_one branch from ab974ef to 08cb3d8 Compare 
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
2 participants
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.
In this take:
DnsNameResolverreturns balancer addresses as aGrpcAttributes.ATTR_LB_ADDRSattribute inResolutionResult.AutoConfiguredLoadBalancerFactorydecides LB policy solely based on parsed service config without looking at resolved addresses. Behavior changes:pick_first, even if there exist balancer addresses.grpclbspecified but not available and no other specified policies available, it will fail without fallback toround_robin.GrpclbLoadBalancerpopulates balancer addresses fromResolvedAddresses's attribute (GrpclbConstants.ATTR_LB_ADDRS) instead of sieving from addresses.Next step after this change is to separate logic for querying SRV record in
DnsNameResolverto agrpclbspecific resolver.