Skip to content

A76 update: Clarify behaviors discovered during implementation #489

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

A76 update: Clarify behaviors discovered during implementation #489

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
37defbb
[A76] Clarify behaviors discovered during implementation
atollena Apr 3, 2025
75008fd
comments from mark, eric
atollena Apr 4, 2025
797f935
review from Doug
atollena May 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions A76-ring-hash-improvements.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,20 @@ if !using_random_hash {
return ring[first_index].picker->Pick(...);
```

This behavior ensures that a single RPC does not cause more than one endpoint to
exit `IDLE` state at a time, and that a request missing the header does not
incur extra latency in the common case where there is already at least one
endpoint in `READY` state. It converges to picking a random endpoint, since each
request may eventually cause a random endpoint to go from `IDLE` to `READY`.
This behavior ensures that a single RPC does not cause more than two endpoints
to exit `IDLE` state, and that a request missing the header does not incur extra
latency in the common case where there is already at least one endpoint in
`READY` state. It converges to picking a random endpoint if the header is not
set, since each request may cause one or two random endpoints to go from `IDLE`
to `READY`: - a first connection attempt if it randomly picks an `IDLE` endpoint
and no endpoint is currently `CONNECTING` - a second connection attempt if the
pick was queued and an endpoint transitioned from `CONNECTING` to `READY`, but
no other endpoint is currently `CONNECTING`.

Because pickers generated with `picker_has_endpoint_in_connecting_state` set to
false may be used by multiple requests until an endpoint transitions to
`CONNECTING` and a the picker is updated, we can still start multiple
simultaneous connection attempts for different RPCs.

### Explicitly setting the endpoint hash key

Expand All @@ -178,6 +187,10 @@ endpoint attribute to the value of [LbEndpoint.Metadata][LbEndpoint.Metadata]
`envoy.lb` `hash_key` field, as described in [Envoy's documentation for the ring
hash load balancer][envoy-ringhash].

If multiple endpoints have the same `hash_key`, then which endpoint is chosen is
Copy link
Member

Choose a reason for hiding this comment

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

I will note that this problem is not strictly new in this gRFC, because it's conceivably possible for there to be hash collisions even in the original design. But I agree that the fact that we're allowing hash keys to be specified manually makes it more likely, so I'm fine with noting this here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

unspecified, and the implementation is free to choose any of the endpoints with
a matching `hash_key`.

### Temporary environment variable protection

Explicitly setting the request hash key will be gated by the
Expand Down Expand Up @@ -230,7 +243,9 @@ considered the following alternative solutions:

## Implementation

Implemented in C-core in https://github.com/grpc/grpc/pull/38312.
Implemented in:
- C-core in https://github.com/grpc/grpc/pull/38312.
- Go: https://github.com/grpc/grpc-go/pull/8159

[A42]: A42-xds-ring-hash-lb-policy.md
[A61]: A61-IPv4-IPv6-dualstack-backends.md
Expand Down