feat(tonic-xds): parse RING_HASH lb policy from CDS (gRFC A42)#2701
Open
madhurishgupta wants to merge 5 commits into
Open
feat(tonic-xds): parse RING_HASH lb policy from CDS (gRFC A42)#2701madhurishgupta wants to merge 5 commits into
madhurishgupta wants to merge 5 commits into
Conversation
Accept lb_policy: RING_HASH and parse ring_hash_lb_config into
LbPolicy::RingHash { min_ring_size, max_ring_size }:
- NACK a hash_function other than XX_HASH.
- NACK any ring size above the 8M ceiling, and NACK min_ring_size >
max_ring_size (matching grpc-go).
- Default unset minimum_ring_size to 1024 and maximum_ring_size to 4096, then
clamp both to the local cap of 4096.
Picker selection from lb_policy is deferred (no live Stack B construction yet).
…policy Move the LbPolicy enum and ring_hash_lb_config parsing out of cluster.rs into a dedicated lb_policy module. The RingHash variant now carries a RingHashSettings payload whose validate() owns the gRFC A42 rules, so the validation lives with the config it produces and is unit-testable on its own. cluster.rs re-exports LbPolicy so existing cluster::LbPolicy paths are unchanged; ClusterResource just delegates to RingHashSettings::validate.
madhurishgupta
force-pushed
the
madhurishgupta/a42-pr3-cds-ring-hash
branch
from
7fdaaad to
a921b5e
Compare
YutaoMa
reviewed
Jun 24, 2026
YutaoMa
reviewed
Jun 24, 2026
Defaulting an unset maximum_ring_size directly to the local cap made a config with min_ring_size in (4096, 8M] and an unset max get NACKed (min > max), which grpc-go accepts: its CDS layer defaults unset max to 8M, so min <= max holds and both are clamped to the cap later. Restore that two-step (default 8M, then clamp to 4096) so the min > max check matches grpc-go.
madhurishgupta
force-pushed
the
madhurishgupta/a42-pr3-cds-ring-hash
branch
from
9c21137 to
e46263d
Compare
YutaoMa
reviewed
Jun 24, 2026
YutaoMa
reviewed
Jun 24, 2026
…ields Make RingHashSettings fields private with getters so the only way to obtain a value is RingHashSettings::validate — invalid (out-of-bounds) values can no longer be constructed directly. Also collapse the duplicate 8M default/ceiling constants into a single RING_HASH_SIZE_CEILING used for both the unset-max default and the ceiling check.
YutaoMa
approved these changes
Jun 25, 2026
Contributor
|
Heads up: since this PR flips ring hash config from NACK to ACK, I'd like to wait until either the LB support is added, or the first version of |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Adds CDS support for the gRFC A42 ring-hash load-balancing policy. A Cluster with lb_policy: RING_HASH is now accepted and its ring_hash_lb_config validated into LbPolicy::RingHash(RingHashSettings { min_ring_size, max_ring_size }).
Validation rules (mirroring grpc-go's balancer/ringhash/config.go ordering — ceiling → min/max → cap):
Tests
Plan