Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core, grpclb: change policy selection strategy for Grpclb policy (take one: eliminate special logic for deciding grpclb policy in core) #6637

Merged
merged 8 commits into from Jan 31, 2020

Conversation

voidzcy
Copy link
Contributor

@voidzcy voidzcy commented Jan 24, 2020

In this take:

  • DnsNameResolver returns balancer addresses as a GrpcAttributes.ATTR_LB_ADDRS attribute in ResolutionResult.
  • AutoConfiguredLoadBalancerFactory decides LB policy solely based on parsed service config without looking at resolved addresses. Behavior changes:
    • If no LB policy is specified in service config, default to pick_first, even if there exist balancer addresses.
    • If grpclb specified but not available and no other specified policies available, it will fail without fallback to round_robin.
  • GrpclbLoadBalancer populates balancer addresses from ResolvedAddresses'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 DnsNameResolver to a grpclb specific resolver.

…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 voidzcy force-pushed the impl/grpclb_selection_change_take_one branch from 4e33ec5 to c090767 Compare Jan 27, 2020
@voidzcy voidzcy force-pushed the impl/grpclb_selection_change_take_one branch from c090767 to da29532 Compare Jan 27, 2020
@voidzcy voidzcy changed the title core, grpclb: change policy selection strategy for Grpclb policy (take one) core, grpclb: change policy selection strategy for Grpclb policy (take one: eliminate special logic for deciding grpclb policy in core) Jan 27, 2020
@voidzcy voidzcy requested review from creamsoup and ejona86 Jan 27, 2020
@voidzcy voidzcy marked this pull request as ready for review Jan 27, 2020
@voidzcy voidzcy force-pushed the impl/grpclb_selection_change_take_one branch from a9d6a40 to b428ae3 Compare Jan 28, 2020
@voidzcy voidzcy force-pushed the impl/grpclb_selection_change_take_one branch from b428ae3 to 5adefa9 Compare Jan 28, 2020
@voidzcy voidzcy requested a review from creamsoup Jan 29, 2020
Copy link
Contributor

@creamsoup creamsoup left a comment

LGTM, can you wait couple days. it would be nice to verify "service config error handling" without this change.

@voidzcy voidzcy force-pushed the impl/grpclb_selection_change_take_one branch 2 times, most recently from 8f38f42 to ab974ef Compare Jan 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants