Skip to content

pickfirstleaf: Fix shuffling of addresses in resolver updates without endpoints#8610

Merged
arjan-bal merged 3 commits into
grpc:masterfrom
arjan-bal:pf-addr-shuffle-bug
Sep 26, 2025
Merged

pickfirstleaf: Fix shuffling of addresses in resolver updates without endpoints#8610
arjan-bal merged 3 commits into
grpc:masterfrom
arjan-bal:pf-addr-shuffle-bug

Conversation

@arjan-bal

@arjan-bal arjan-bal commented Sep 25, 2025

Copy link
Copy Markdown
Contributor

The new pick_first, which is the default, doesn't shuffle the addresses at all for resolver updates that are missing the Endpoints field. This change fixes that. Since gRPC automatically sets the the missing Endpoints, occurrence of this bug should be uncommon in practice.

RELEASE NOTES:

  • balancer/pick_first: When configured, shuffle addresses in resolver updates that lack endpoints. Since gRPC automatically adds endpoints to resolver updates, this bug should only affect implementers of custom LB policies that use pick_first for delegation but don't forward the endpoints.

@arjan-bal arjan-bal added Type: Bug Area: Resolvers/Balancers Includes LB policy & NR APIs, resolver/balancer/picker wrappers, LB policy impls and utilities. labels Sep 25, 2025
@arjan-bal arjan-bal added this to the 1.77 Release milestone Sep 25, 2025
@codecov

codecov Bot commented Sep 25, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.16%. Comparing base (8ae3c07) to head (1adccbc).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
balancer/pickfirst/pickfirst.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8610      +/-   ##
==========================================
+ Coverage   82.11%   82.16%   +0.04%     
==========================================
  Files         415      415              
  Lines       40692    40697       +5     
==========================================
+ Hits        33416    33439      +23     
+ Misses       5887     5881       -6     
+ Partials     1389     1377      -12     
Files with missing lines Coverage Δ
balancer/pickfirst/pickfirstleaf/pickfirstleaf.go 88.25% <100.00%> (+0.58%) ⬆️
balancer/pickfirst/pickfirst.go 34.56% <0.00%> (ø)

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

}

pfBuilder := balancer.Get(pfbalancer.Name)
shffleConfig, err := pfBuilder.(balancer.ConfigParser).ParseConfig(json.RawMessage(`{ "shuffleAddressList": true }`))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lol, here and in the next statement, prefer adding the u back into shuffle. Saving one character in the variable name is not going to be very useful :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed to using complete words.

if err != nil {
t.Fatal(err)
}
activeCfg := noShffleConfig

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: remove this line since this assignment is anyways happening on line 521.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed to only declaring the variable, but not initializing here.

@easwars easwars assigned arjan-bal and unassigned easwars and dfawley Sep 25, 2025
@arjan-bal arjan-bal merged commit bb71072 into grpc:master Sep 26, 2025
15 checks passed
@arjan-bal arjan-bal deleted the pf-addr-shuffle-bug branch September 26, 2025 05:38
arjan-bal added a commit to arjan-bal/grpc-go that referenced this pull request Oct 1, 2025
… endpoints (grpc#8610)

The new `pick_first`, which is the default, doesn't shuffle the
addresses at all for resolver updates that are missing the `Endpoints`
field. This change fixes that. Since [gRPC automatically sets the the
missing
`Endpoints`](https://github.com/grpc/grpc-go/blob/1059e84f885bf7ed65b3b1a4fbe914360d8ab5b1/resolver_wrapper.go#L136-L138),
occurrence of this bug should be uncommon in practice.

RELEASE NOTES:
* balancer/pick_first: When configured, shuffle addresses in resolver
updates that lack endpoints. Since gRPC automatically adds endpoints to
resolver updates, this bug should only affect implementers of custom LB
policies that use pick_first for delegation but don't forward the
endpoints.
arjan-bal added a commit that referenced this pull request Oct 3, 2025
Original PRs: #8610, #8615

RELEASE NOTES:
* balancer/pick_first:
* Fix bug that can cause balancer to get stuck in `IDLE` state on
backend address change.
* When configured, shuffle addresses in resolver updates that lack
endpoints. Since gRPC automatically adds endpoints to resolver updates,
this bug should only affect implementers of custom LB policies that use
pick_first for delegation but don't forward the endpoints.
Pranjali-2501 pushed a commit to Pranjali-2501/grpc-go that referenced this pull request Oct 6, 2025
… endpoints (grpc#8610)

The new `pick_first`, which is the default, doesn't shuffle the
addresses at all for resolver updates that are missing the `Endpoints`
field. This change fixes that. Since [gRPC automatically sets the the
missing
`Endpoints`](https://github.com/grpc/grpc-go/blob/1059e84f885bf7ed65b3b1a4fbe914360d8ab5b1/resolver_wrapper.go#L136-L138),
occurrence of this bug should be uncommon in practice.

RELEASE NOTES:
* balancer/pick_first: When configured, shuffle addresses in resolver
updates that lack endpoints. Since gRPC automatically adds endpoints to
resolver updates, this bug should only affect implementers of custom LB
policies that use pick_first for delegation but don't forward the
endpoints.
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 26, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Area: Resolvers/Balancers Includes LB policy & NR APIs, resolver/balancer/picker wrappers, LB policy impls and utilities. Type: Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants