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: Optional address shuffle in PickFirstLoadBalancer #10110

Merged
merged 5 commits into from Apr 28, 2023

Conversation

temawi
Copy link
Contributor

@temawi temawi commented Apr 28, 2023

If provided with the new PickFirstLoadBalancerConfig, PickFirstLoadBalancer will shuffle the list of addresses it receives from the name resolver.

If provided with the new PickFirstLoadBalancerConfig,
PickFirstLoadBalancer will shuffle the list of addresses it receives
from the name resolver.
Copy link
Contributor

@larry-safran larry-safran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add tests

  • parseLoadBalancingPolicyConfig where enablePickFirstConfig==false and shuffle==true
  • picks where enablePickFirstConfig==true and shuffle==false

@temawi
Copy link
Contributor Author

temawi commented Apr 28, 2023

Should add tests

  • parseLoadBalancingPolicyConfig where enablePickFirstConfig==false and shuffle==true
  • picks where enablePickFirstConfig==true and shuffle==false

I moved the flag checking to the provider so these exact conditions don't exist in the LB anymore. But I think between the LB and provider tests the bases are covered.

@larry-safran
Copy link
Contributor

You're missing the copyright in the new test file.

@temawi temawi merged commit 3c01bfe into grpc:master Apr 28, 2023
14 checks passed
@temawi temawi deleted the pf-shuffle branch April 28, 2023 22:50
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2023
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