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

A42 update: io.grpc.channel_id should be initialized with a random number #320

Merged
merged 3 commits into from
Sep 15, 2022
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 9 additions & 6 deletions A42-xds-ring-hash-lb-policy.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,15 @@ Here is how gRPC will handle each type of hash policy:
the way that Envoy does. However, we will support one special filter state
[`key`](https://github.com/envoyproxy/envoy/blob/2443032526cf6e50d63d35770df9473dd0460fc0/api/envoy/config/route/v3/route_components.proto#L703)
called `io.grpc.channel_id`, which will hash to the same value for all
requests on a given gRPC channel. This can be used in similar situations
to where Envoy uses `connection_properties` to hash on the source IP address.
(Note that we do not recommend that applications create multiple gRPC
channels to the same virtual host, but if you do that, then the
behavior here will not be exactly the same as using `connection_properties`,
because each channel may use a different endpoint.)
requests on a given gRPC channel. In order to facilitate an even selection
of backends across different channels (which may or may not be in the same
process or machine), the value of `io.grpc.channel_id` should be initialized
with a uniform random number. This can be used in similar situations to where
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this instead?

Suggested change
with a uniform random number. This can be used in similar situations to where
with a random number from a uniform distribution. This can be used in similar situations to where

"uniform random number" could be misread as "generate a single random number and use it for all the channel ids", but I think you're trying to say that each client id should be different (and if you want a uniform distribution across backends, the random number needs to be from a uniform distribution so that hashing doesn't get skewed).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could see that. Changed to your wording.

Envoy uses `connection_properties` to hash on the source IP address. (Note
that we do not recommend that applications create multiple gRPC channels to
the same virtual host, but if you do that, then the behavior here will not be
exactly the same as using `connection_properties`, because each channel
may use a different endpoint.)

#### XdsClient Changes

Expand Down