A42 update: io.grpc.channel_id should be initialized with a random number #320
Conversation
A42-xds-ring-hash-lb-policy.md
Outdated
| (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.) | ||
| because each channel may use a different endpoint.). In order to facilitate |
There was a problem hiding this comment.
(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 usingconnection_properties,
because each channel may use a different endpoint.)
Did we want that behavior? Because we could initialize this ID globally to a process if desired.
Also: nit: no period after the parenthetical sentence. But should this sentence come before it, maybe?
There was a problem hiding this comment.
Did we want that behavior? Because we could initialize this ID globally to a process if desired.
Changed wording to note that we do want even backend distribution across channels, which may or may not be in the same process or machine (i.e. we want even distribution across all channels).
We know of use cases that do use multiple channels within a process, and for those use cases, each channel should have affinity to its own backend (but hot spotting one backend is undesirable for them).
Also: nit: no period after the parenthetical sentence. But should this sentence come before it, maybe?
Moved things around to keep the parenthetical sentence at the end.
A42-xds-ring-hash-lb-policy.md
Outdated
| 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 |
There was a problem hiding this comment.
Perhaps this instead?
| 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).
There was a problem hiding this comment.
I could see that. Changed to your wording.
Compliments some changes that have been made or are being made: