Skip to content

Conversation

@ejona86
Copy link
Member

@ejona86 ejona86 commented Jan 23, 2026

There's two commits here with descriptions in each; take a look at each individually and I'll keep them separate when merging.

There will be a gRFC for this, but apparently it hasn't been created yet (gRFC A113 grpc/proposal#535) and I didn't want to wait longer for it to be created before starting review, because the plan is to delay the 1.79.0 release for this in Java and Go. We will want to merge this ASAP (and backport at our convenience), but we must wait until the gRFC is merged before publishing the 1.79.0 release. I suggest we leave TODO:release blocker label here until the gRFC is merged. (Kannan, I shared a doc and an email thread with you to give you some context; mostly for what problem is being solved, and not the specific solution here. The specific solution here is split across a lot of comments, so you're best off waiting for the gRFC to see something documenting it.)

While doing this I've noticed lots of things to fix with how weights are handled. I've basically ignored them at the moment, only trying to make sure that I don't make things worse. I'll be doing a follow-up to fix more weight handling, but I will not be trying to backport it.

@ejona86 ejona86 requested a review from kannanjgithub January 23, 2026 23:25
@ejona86 ejona86 added the TODO:release blocker Issue/PR is important enough to delay the release. Removed after release issues resolved label Jan 23, 2026
The prior uniform shuffle in pick_first will send uniform load across
clients. When endpoints have weights, we'd desire for endpoints to be
selected proportionally to their weight.

The server weight attribute has to move out of xDS to be seen by
pick-first, but it is kept as internal for now. Since xDS is the only
thing that sets weights, the behavior change is only visible to xDS.

See gRFC A113
@ejona86 ejona86 force-pushed the pf-weighted-shuffle branch from 78a0f8f to 338846f Compare January 27, 2026 22:04
@ejona86
Copy link
Member Author

ejona86 commented Jan 27, 2026

The force-push was me adding "gRFC A113" in a comment and in the commit descriptions.

@ejona86 ejona86 added the TODO:backport PR needs to be backported. Removed after backport complete label Jan 27, 2026
Previously, the number of endpoints in a locality would skew how much
traffic was sent to that locality. Also, if endpoints in localities had
wildly different weights, that would impact cross-locality weighting.

For example, consider:
  LocalityA weight=1 endpointWeights=[100, 100, 100, 100]
  LocalityB weight=1 endpointWeights=[1]

The endpoint in LocalityB should have an endpoint weight that is half
the total sum of endpoint weights, in order to receive half the traffic.
But the multiple endpoints in LocalityA would cause it to get 4x the
traffic and the endpoint weights in LocalityA causes them to get 100x
the traffic.

See gRFC A113
@ejona86 ejona86 force-pushed the pf-weighted-shuffle branch from 338846f to 4305786 Compare January 27, 2026 22:31
@ejona86
Copy link
Member Author

ejona86 commented Jan 27, 2026

Aaand that force-push changed new HashMap<>(sortedPriorityNames.size()) to new HashMap<>(sortedPriorityNames.size() * 2), since the value is the table size, not the expected capacity.

@ejona86 ejona86 merged commit 02cb1de into grpc:master Jan 28, 2026
17 checks passed
@ejona86 ejona86 deleted the pf-weighted-shuffle branch January 28, 2026 00:21
@ejona86 ejona86 removed the TODO:backport PR needs to be backported. Removed after backport complete label Jan 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

TODO:release blocker Issue/PR is important enough to delay the release. Removed after release issues resolved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants