From e55847642953942e625e81698731b8f543c00d64 Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Mon, 15 Jan 2024 14:38:50 +0100 Subject: [PATCH 01/11] A76: Improvements to the Ring Hash LB Policy --- A76-ring-hash-improvements.md | 164 ++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 A76-ring-hash-improvements.md diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md new file mode 100644 index 000000000..e97c9410d --- /dev/null +++ b/A76-ring-hash-improvements.md @@ -0,0 +1,164 @@ +A76: Improvements to the Ring Hash LB Policy +---- +* Author(s): @atollena +* Approver: ? +* Status: Draft +* Implemented in: +* Last updated: 2024-01-15 +* Discussion at: TODO + +## Abstract + +This proposal describes two improvements to the `ring_hash` load balancing policy: + +1. The ability to use ring hash without xDS, by extending the policy + configuration to define the request + [metadata](https://grpc.io/docs/what-is-grpc/core-concepts/#metadata) to use + as the request hash key. +2. The ability to specify endpoint hash keys explicitly, instead of always using + the endpoint IP address, to allow replacing them at the same place on the + ring when logical endpoints change their IP address. + +## Background + +### Terminology + +Those two terms that are used throughout this gRFC: + +* The *request hash key*, after being hashed, defines where a given request is + to be placed on the ring in order to find the closest endpoints. +* The *endpoint hash key*, after being hashed, defines where an endpoint is to + be placed on the ring. + +### Using ring hash without xDS by explicitly setting the request hash key + +gRPC supports the `ring_hash` load balancing policy for consistent hashing. This +policy currently requires using xDS for configuration because users have no +other way to configure the hash for a request but to use the route configuration +`hash_policy` field in the `RouteAction` route configuration. This makes the +`ring_hash` policy unusable without an xDS infrastructure in place. + +This proposal extends the configuration of `ring_hash` policy to specify a +metadata key. The value associated with this metadata key will be used as the +request hash key if present. This will make it possible to use `ring_hash` by +configuring it entirely in the [service +config](https://github.com/grpc/grpc/blob/master/doc/service_config.md). If this +configuration is omitted, we will preserve the current behavior of using the xDS +hash policy. + +### Using an explicit endpoint hash key + +Another limitation of the current `ring_hash` load balancing policy is that it +always hashes the endpoint IP address to place the endpoints on the ring. In +some scenario, this choice is not ideal: for example, [Kubernetes +Statefulsets](https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/) +offer a way to configure workloads with sticky identity such that endpoints keep +storage and network identifier across restarts. However, the IP address may +change across restarts. After a deployment, if all IP addresses have changed, +then a completely new ring has to be constructed, even though it may have been +desirable to keep the ring unchanged based on the Statefulsets identities, so +that each instance stays at the same location on the ring. + +Envoy offers a solution to control endpoint hashes independently of IP +addresses. This mechanism uses the `"envoy.lb"` +[LbEndpoint.Metadata](https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/endpoint/v3/endpoint_components.proto#envoy-v3-api-field-config-endpoint-v3-lbendpoint-metadata) +field `hash_key` value available in EDS instead of the endpoint IP address, as +described in [the Envoy documentation for ring +hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash). +This proposal adds support for setting the endpoint hash key explicitly via EDS +by reusing the configuration implemented in Envoy. To retain the advantage of +being able to use `ring_hash` without xDS, custom gRPC name resolvers will be +able to set this endpoint attribute through the language specific resolver +attribute interface. + +### Related Proposals: + +This proposal extends the following existing gRFC: + +* [gRFC A42: xDS Ring Hash LB Policy](A42-xds-ring-hash-lb-policy.md) + +## Proposal + +### Explicitly setting the request hash key + +A new string field `request_metadata_key` will be added to the `ring_hash` +policy config. The ring hash policy will be modified as follow: if this +configuration field is not empty, at pick time, the request hash key will be set +to the value associated with this metadata key. If this configuration field is +not set, then the request hash key will be based on the xDS hash policy in RDS +(current behavior). If the field is omitted but the xDS configuration does not +provide the hash key, then the picker will generate a random hash for it. If the +request has no value associated with the metadata key defined in the +configuration, then the picker will generate a random hash for it. The use of a +random hash key will effectively sends the request to a random endpoint. + +### Explicitly setting the endpoint hash key + +The `ring_hash` policy will be changed such that the hash key used for placing +each endpoint on the ring will be extracted from a pre-defined name resolver +attribute called `hash_key`. If this attribute is set, then the endpoint is +placed on the ring by hashing its value. If this attribute is not set, then the +endpoint IP address is used (current behavior). The location of an existing +endpoint on the ring changes if its `hash_key` name resolver attribute changes. + +The xDS resolver will be changed so that when converting EDS responses to +resolver endpoints, it will set the `hash_key` name resolver attribute to the +value of +[LbEndpoint.Metadata](https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/endpoint/v3/endpoint_components.proto#envoy-v3-api-field-config-endpoint-v3-lbendpoint-metadata) +`envoy.lb` `hash_key` field, as described in [Envoy's documentation for the ring +hash load +balancer](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash). + +### LB Policy Config Changes + +After the addition of this field, the `ring_hash` LB policy config will be: + + message RingHashLoadBalancingConfig { + // A client-side cap will limit these values. If either of these values + // are greater than the client-side cap, they will be treated as the + // client-side cap. The default cap is 4096. + uint64 min_ring_size = 1; // Optional, defaults to 1024. + uint64 max_ring_size = 2; // Optional, defaults to 4096, max is 8M. + + string request_metadata_key = 3; // Optional, defaults to the empty string. + } + + +### Temporary environment variable protection + +Explicitly setting the request hash key cannot possibly lead to problem with +existing deployment because the new behavior requires setting a load balancing +policy configuration field that did not exist before. Therefore, it is not gated +behind an environment variable. + +The second behavior change will be enabled by the +`GRPC_EXPERIMENTAL_XDS_RING_HASH_ENDPOINT_HASH_KEY` environment variable. This +will protect from the case where an xDS control plane is already setting the +`LbEndpoint.Metadata` `envoy.lb` `hash_key` field, in which case deploying this +new behavior would churn all endpoint hash keys, which could lead to +problems. This environment variable will be removed once the feature has proven +stable. + +## Rationale + +We originally proposed using language specific interfaces to set the request +hash key. The advantage would have been that the request hash key would not have +to be exposed through gRPC metadata. However, this would have required defining +language specific APIs, which would increase the complexity of this change. + +We also discussed the option of exposing all `LbEndpoint.metadata` from EDS +through name resolver attributes, instead of only extracting the specific +`hash_key` attribute, so as to make them available to custom LB policies. We +decided to keep only extract `hash_key` to limit the scope of this gRFC. + +## Implementation + +[A description of the steps in the implementation, who will do them, and when. +If a particular language is going to get the implementation first, this section +should list the proposed order.] + +Implemented in Go: XXX + +## Open issues (if applicable) + +N/A From 54074388ca49e7c8eb1060af238ce98a63ad9daa Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Mon, 22 Jan 2024 09:23:17 +0100 Subject: [PATCH 02/11] Comments from Sergey --- A76-ring-hash-improvements.md | 112 +++++++++++++++++++--------------- 1 file changed, 62 insertions(+), 50 deletions(-) diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md index e97c9410d..5f2449885 100644 --- a/A76-ring-hash-improvements.md +++ b/A76-ring-hash-improvements.md @@ -12,23 +12,19 @@ A76: Improvements to the Ring Hash LB Policy This proposal describes two improvements to the `ring_hash` load balancing policy: 1. The ability to use ring hash without xDS, by extending the policy - configuration to define the request - [metadata](https://grpc.io/docs/what-is-grpc/core-concepts/#metadata) to use - as the request hash key. -2. The ability to specify endpoint hash keys explicitly, instead of always using - the endpoint IP address, to allow replacing them at the same place on the - ring when logical endpoints change their IP address. + configuration to define the [request metadata][metadata] to use as the + request hash key. +2. The ability to specify endpoint hash keys explicitly, instead of hashing the + endpoint IP address. ## Background ### Terminology -Those two terms that are used throughout this gRFC: - * The *request hash key*, after being hashed, defines where a given request is to be placed on the ring in order to find the closest endpoints. -* The *endpoint hash key*, after being hashed, defines where an endpoint is to - be placed on the ring. +* The *endpoint hash key*, after being hashed, determines the locations of an + endpoint on the ring. ### Using ring hash without xDS by explicitly setting the request hash key @@ -41,8 +37,7 @@ other way to configure the hash for a request but to use the route configuration This proposal extends the configuration of `ring_hash` policy to specify a metadata key. The value associated with this metadata key will be used as the request hash key if present. This will make it possible to use `ring_hash` by -configuring it entirely in the [service -config](https://github.com/grpc/grpc/blob/master/doc/service_config.md). If this +configuring it entirely in the [service config][service-config]. If this configuration is omitted, we will preserve the current behavior of using the xDS hash policy. @@ -61,53 +56,64 @@ that each instance stays at the same location on the ring. Envoy offers a solution to control endpoint hashes independently of IP addresses. This mechanism uses the `"envoy.lb"` -[LbEndpoint.Metadata](https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/endpoint/v3/endpoint_components.proto#envoy-v3-api-field-config-endpoint-v3-lbendpoint-metadata) -field `hash_key` value available in EDS instead of the endpoint IP address, as -described in [the Envoy documentation for ring -hash](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash). -This proposal adds support for setting the endpoint hash key explicitly via EDS -by reusing the configuration implemented in Envoy. To retain the advantage of -being able to use `ring_hash` without xDS, custom gRPC name resolvers will be -able to set this endpoint attribute through the language specific resolver -attribute interface. - -### Related Proposals: +[LbEndpoint.Metadata][LbEndpoint.Metadata] field `hash_key` value available in +EDS instead of the endpoint IP address, as described in [the Envoy documentation +for ring hash][envoy-ringhash]. This proposal adds support for setting the +endpoint hash key explicitly via EDS by reusing the configuration mechanism +implemented in Envoy. To retain the advantage of being able to use `ring_hash` +without xDS, custom gRPC name resolvers will be able to set this endpoint +attribute through the language specific resolver attribute interface. + +### Related Proposals: This proposal extends the following existing gRFC: -* [gRFC A42: xDS Ring Hash LB Policy](A42-xds-ring-hash-lb-policy.md) +* [gRFC A42: xDS Ring Hash LB Policy][A42] ## Proposal ### Explicitly setting the request hash key A new string field `request_metadata_key` will be added to the `ring_hash` -policy config. The ring hash policy will be modified as follow: if this -configuration field is not empty, at pick time, the request hash key will be set -to the value associated with this metadata key. If this configuration field is -not set, then the request hash key will be based on the xDS hash policy in RDS -(current behavior). If the field is omitted but the xDS configuration does not -provide the hash key, then the picker will generate a random hash for it. If the -request has no value associated with the metadata key defined in the -configuration, then the picker will generate a random hash for it. The use of a -random hash key will effectively sends the request to a random endpoint. +policy config. The ring hash policy will be modified as follows: + +Upon loading the load balancing config, if the `request_metadata_key` field +contains a value that is not a valid metadata key, then the configuration is +rejected. If the `request_metadata_key` refers to a binary metadata (prefixed +with `-bin`), the configuration is also rejected. + +At pick time: +- If `request_metadata_key` is not empty, and the request metadata has a +non-empty value, then the request hash key will be set to this value. If +`request_metadata_key` contains multiple values, then values are joined with a +coma `,` character between each value before hashing. +- If `request_metadata_key` is not empty, and the request has no value or an +empty value associated with the metadata key defined, then the picker will +generate a random hash for it. The use of a random hash key will effectively +sends the request to a random endpoint. +- If `request_metadata_key` is not set or empty, then the request hash key will +be based on the xDS hash policy in RDS, which keeps the existing LB +configuration for ring hash working as before with xDS. +- If `request_metadata_key` is not set or empty but the xDS configuration does +not provide the hash key, then the picker will generate a random hash for it to +select a random endpoint, which matches the current behavior for xDS. + ### Explicitly setting the endpoint hash key -The `ring_hash` policy will be changed such that the hash key used for placing -each endpoint on the ring will be extracted from a pre-defined name resolver -attribute called `hash_key`. If this attribute is set, then the endpoint is -placed on the ring by hashing its value. If this attribute is not set, then the -endpoint IP address is used (current behavior). The location of an existing -endpoint on the ring changes if its `hash_key` name resolver attribute changes. +The `ring_hash` policy will be changed such that the hash key used for +determining the locations of each endpoint on the ring will be extracted from a +pre-defined name resolver attribute called `hash_key`. If this attribute is set, +then the endpoint is placed on the ring by hashing its value. If this attribute +is not set or empty, then the endpoint IP address is used (current +behavior). The location of an existing endpoint on the ring changes if its +`hash_key` name resolver attribute changes. The xDS resolver will be changed so that when converting EDS responses to resolver endpoints, it will set the `hash_key` name resolver attribute to the -value of -[LbEndpoint.Metadata](https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/endpoint/v3/endpoint_components.proto#envoy-v3-api-field-config-endpoint-v3-lbendpoint-metadata) -`envoy.lb` `hash_key` field, as described in [Envoy's documentation for the ring -hash load -balancer](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash). +value of [LbEndpoint.Metadata][LbEndpoint.Metadata] `envoy.lb` `hash_key` field, +as described in [Envoy's documentation for the ring hash load +balancer](envoy-ring-hash). ### LB Policy Config Changes @@ -135,16 +141,16 @@ The second behavior change will be enabled by the `GRPC_EXPERIMENTAL_XDS_RING_HASH_ENDPOINT_HASH_KEY` environment variable. This will protect from the case where an xDS control plane is already setting the `LbEndpoint.Metadata` `envoy.lb` `hash_key` field, in which case deploying this -new behavior would churn all endpoint hash keys, which could lead to -problems. This environment variable will be removed once the feature has proven -stable. +new behavior would churn all endpoint hash keys. This environment variable will +be removed once the feature has proven stable. ## Rationale We originally proposed using language specific interfaces to set the request hash key. The advantage would have been that the request hash key would not have -to be exposed through gRPC metadata. However, this would have required defining -language specific APIs, which would increase the complexity of this change. +to be exposed through gRPC outgoing metadata. However, this would have required +defining language specific APIs, which would increase the complexity of this +change. We also discussed the option of exposing all `LbEndpoint.metadata` from EDS through name resolver attributes, instead of only extracting the specific @@ -157,8 +163,14 @@ decided to keep only extract `hash_key` to limit the scope of this gRFC. If a particular language is going to get the implementation first, this section should list the proposed order.] -Implemented in Go: XXX +Will provide an implementation in Go. ## Open issues (if applicable) N/A + +[A42]: A42-xds-ring-hash-lb-policy.md +[envoy-ringhash]: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash +[metadata]: https://grpc.io/docs/what-is-grpc/core-concepts/#metadata +[service-config]: https://github.com/grpc/grpc/blob/master/doc/service_config.md +[LbEndpoint.Metadata]: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/endpoint/v3/endpoint_components.proto#envoy-v3-api-field-config-endpoint-v3-lbendpoint-metadata From cc3af7f77372805823bbb6d6f3b9f7d185a264f1 Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Mon, 22 Jan 2024 21:51:43 +0100 Subject: [PATCH 03/11] comments from Michael --- A76-ring-hash-improvements.md | 53 +++++++++++++++-------------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md index 5f2449885..d6f57f11a 100644 --- a/A76-ring-hash-improvements.md +++ b/A76-ring-hash-improvements.md @@ -79,46 +79,47 @@ policy config. The ring hash policy will be modified as follows: Upon loading the load balancing config, if the `request_metadata_key` field contains a value that is not a valid metadata key, then the configuration is -rejected. If the `request_metadata_key` refers to a binary metadata (prefixed +rejected. If the `request_metadata_key` refers to a binary metadata (suffixed with `-bin`), the configuration is also rejected. At pick time: - If `request_metadata_key` is not empty, and the request metadata has a non-empty value, then the request hash key will be set to this value. If `request_metadata_key` contains multiple values, then values are joined with a -coma `,` character between each value before hashing. +comma `,` character between each value before hashing. - If `request_metadata_key` is not empty, and the request has no value or an empty value associated with the metadata key defined, then the picker will generate a random hash for it. The use of a random hash key will effectively sends the request to a random endpoint. -- If `request_metadata_key` is not set or empty, then the request hash key will -be based on the xDS hash policy in RDS, which keeps the existing LB -configuration for ring hash working as before with xDS. -- If `request_metadata_key` is not set or empty but the xDS configuration does -not provide the hash key, then the picker will generate a random hash for it to -select a random endpoint, which matches the current behavior for xDS. +- If `request_metadata_key` is empty, then the request hash key will be based on +the xDS hash policy in RDS, which keeps the existing LB configuration for ring +hash working as before with xDS. +- If `request_metadata_key` is empty but the xDS configuration does not provide +the hash key, then the picker will generate a random hash for it to select a +random endpoint, which matches the current behavior for xDS. ### Explicitly setting the endpoint hash key The `ring_hash` policy will be changed such that the hash key used for determining the locations of each endpoint on the ring will be extracted from a -pre-defined name resolver attribute called `hash_key`. If this attribute is set, -then the endpoint is placed on the ring by hashing its value. If this attribute -is not set or empty, then the endpoint IP address is used (current -behavior). The location of an existing endpoint on the ring changes if its -`hash_key` name resolver attribute changes. - -The xDS resolver will be changed so that when converting EDS responses to -resolver endpoints, it will set the `hash_key` name resolver 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-ring-hash). +pre-defined endpoint attribute called `hash_key`. If this attribute is set, then +the endpoint is placed on the ring by hashing its value. If this attribute is +not set or empty, then the endpoint IP address is hashed, matching the current +behavior. The locations of an existing endpoint on the ring is updated if its +`hash_key` endpoint attribute changes. + +The cluster resolver LB policy responsible for translating EDS responses into +resolver endpoints will be changed to set the `hash_key` 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-ring-hash]. ### LB Policy Config Changes After the addition of this field, the `ring_hash` LB policy config will be: +```proto message RingHashLoadBalancingConfig { // A client-side cap will limit these values. If either of these values // are greater than the client-side cap, they will be treated as the @@ -126,9 +127,9 @@ After the addition of this field, the `ring_hash` LB policy config will be: uint64 min_ring_size = 1; // Optional, defaults to 1024. uint64 max_ring_size = 2; // Optional, defaults to 4096, max is 8M. - string request_metadata_key = 3; // Optional, defaults to the empty string. + string request_metadata_key = 3; // Optional, unused if unset. } - +``` ### Temporary environment variable protection @@ -141,7 +142,7 @@ The second behavior change will be enabled by the `GRPC_EXPERIMENTAL_XDS_RING_HASH_ENDPOINT_HASH_KEY` environment variable. This will protect from the case where an xDS control plane is already setting the `LbEndpoint.Metadata` `envoy.lb` `hash_key` field, in which case deploying this -new behavior would churn all endpoint hash keys. This environment variable will +new behavior would change all endpoint hash keys. This environment variable will be removed once the feature has proven stable. ## Rationale @@ -159,16 +160,8 @@ decided to keep only extract `hash_key` to limit the scope of this gRFC. ## Implementation -[A description of the steps in the implementation, who will do them, and when. -If a particular language is going to get the implementation first, this section -should list the proposed order.] - Will provide an implementation in Go. -## Open issues (if applicable) - -N/A - [A42]: A42-xds-ring-hash-lb-policy.md [envoy-ringhash]: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash [metadata]: https://grpc.io/docs/what-is-grpc/core-concepts/#metadata From 01449d7d1c06d7df857c46967c23f7aedd4dc3c2 Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Tue, 23 Jan 2024 09:18:33 +0100 Subject: [PATCH 04/11] feedback from Mark --- A76-ring-hash-improvements.md | 108 +++++++++++++++------------------- 1 file changed, 49 insertions(+), 59 deletions(-) diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md index d6f57f11a..e1e73f8f6 100644 --- a/A76-ring-hash-improvements.md +++ b/A76-ring-hash-improvements.md @@ -1,19 +1,20 @@ A76: Improvements to the Ring Hash LB Policy ---- -* Author(s): @atollena -* Approver: ? +* Author(s): atollena +* Approver: markdroth * Status: Draft * Implemented in: -* Last updated: 2024-01-15 -* Discussion at: TODO +* Last updated: 2024-01-23 +* Discussion at: https://groups.google.com/g/grpc-io/c/ZKI1RIF0e_s/m/oBXqOFb0AQAJ ## Abstract -This proposal describes two improvements to the `ring_hash` load balancing policy: +This proposal describes two improvements to the `ring_hash` load balancing +policy: 1. The ability to use ring hash without xDS, by extending the policy - configuration to define the [request metadata][metadata] to use as the - request hash key. + configuration to define the [request header][header] to use as the request + hash key. 2. The ability to specify endpoint hash keys explicitly, instead of hashing the endpoint IP address. @@ -35,11 +36,9 @@ other way to configure the hash for a request but to use the route configuration `ring_hash` policy unusable without an xDS infrastructure in place. This proposal extends the configuration of `ring_hash` policy to specify a -metadata key. The value associated with this metadata key will be used as the -request hash key if present. This will make it possible to use `ring_hash` by -configuring it entirely in the [service config][service-config]. If this -configuration is omitted, we will preserve the current behavior of using the xDS -hash policy. +header to hash. This will make it possible to use `ring_hash` by configuring it +entirely in the [service config][service-config]. If this configuration is +omitted, we will preserve the current behavior of using the xDS hash policy. ### Using an explicit endpoint hash key @@ -74,30 +73,33 @@ This proposal extends the following existing gRFC: ### Explicitly setting the request hash key -A new string field `request_metadata_key` will be added to the `ring_hash` -policy config. The ring hash policy will be modified as follows: +A new field `request_hash_header` will be added to the `ring_hash` policy +config: -Upon loading the load balancing config, if the `request_metadata_key` field -contains a value that is not a valid metadata key, then the configuration is -rejected. If the `request_metadata_key` refers to a binary metadata (suffixed -with `-bin`), the configuration is also rejected. +```proto + message RingHashLoadBalancingConfig { + // (existing fields omitted) + string request_hash_header = 3; + } +``` + +Upon loading the load balancing config, if the `request_hash_header` field +contains a value that is not a valid header name, then the configuration is +rejected. If the `request_hash_header` refers to a binary header (suffixed with +`-bin`), the configuration is also rejected. At pick time: -- If `request_metadata_key` is not empty, and the request metadata has a -non-empty value, then the request hash key will be set to this value. If -`request_metadata_key` contains multiple values, then values are joined with a -comma `,` character between each value before hashing. -- If `request_metadata_key` is not empty, and the request has no value or an -empty value associated with the metadata key defined, then the picker will -generate a random hash for it. The use of a random hash key will effectively -sends the request to a random endpoint. -- If `request_metadata_key` is empty, then the request hash key will be based on +- If `request_hash_header` is not empty, and the header has a non-empty value, +then the request hash key will be set to this value. If `request_hash_header` +contains multiple values, then values are joined with a comma `,` character +between each value before hashing. +- If `request_hash_header` is not empty, and the request has no value or an +empty value associated with the header defined, then the picker will generate a +random hash for it. The use of a random hash key will effectively sends the +request to a random endpoint. +- If `request_hash_header` is empty, then the request hash key will be based on the xDS hash policy in RDS, which keeps the existing LB configuration for ring hash working as before with xDS. -- If `request_metadata_key` is empty but the xDS configuration does not provide -the hash key, then the picker will generate a random hash for it to select a -random endpoint, which matches the current behavior for xDS. - ### Explicitly setting the endpoint hash key @@ -109,28 +111,12 @@ not set or empty, then the endpoint IP address is hashed, matching the current behavior. The locations of an existing endpoint on the ring is updated if its `hash_key` endpoint attribute changes. -The cluster resolver LB policy responsible for translating EDS responses into -resolver endpoints will be changed to set the `hash_key` 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 +For xDS, the cluster resolver LB policy responsible for translating EDS +responses into resolver endpoints will be changed to set the `hash_key` 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-ring-hash]. -### LB Policy Config Changes - -After the addition of this field, the `ring_hash` LB policy config will be: - -```proto - message RingHashLoadBalancingConfig { - // A client-side cap will limit these values. If either of these values - // are greater than the client-side cap, they will be treated as the - // client-side cap. The default cap is 4096. - uint64 min_ring_size = 1; // Optional, defaults to 1024. - uint64 max_ring_size = 2; // Optional, defaults to 4096, max is 8M. - - string request_metadata_key = 3; // Optional, unused if unset. - } -``` - ### Temporary environment variable protection Explicitly setting the request hash key cannot possibly lead to problem with @@ -138,18 +124,22 @@ existing deployment because the new behavior requires setting a load balancing policy configuration field that did not exist before. Therefore, it is not gated behind an environment variable. -The second behavior change will be enabled by the -`GRPC_EXPERIMENTAL_XDS_RING_HASH_ENDPOINT_HASH_KEY` environment variable. This -will protect from the case where an xDS control plane is already setting the -`LbEndpoint.Metadata` `envoy.lb` `hash_key` field, in which case deploying this -new behavior would change all endpoint hash keys. This environment variable will -be removed once the feature has proven stable. +Adding support for the hash_key in xDS endpoint metadata could potentially break +existing clients whose control plane is setting this key, because upgrading the +client to a new version of gRPC would automatically cause the key to start being +used. We expect that this change will not cause problems for most users, but +just in case there is a problem, we will provide a migration story by supporting +a temporary mechanism to tell gRPC to ignore the `hash_key` endpoint +metadata. This will be enabled by setting the +`GRPC_XDS_ENDPOINT_HASH_KEY_BACKWARD_COMPAT` environment variable to true. This +mechanism will be supported for a couple of gRPC releases but will be removed in +the long run. ## Rationale We originally proposed using language specific interfaces to set the request hash key. The advantage would have been that the request hash key would not have -to be exposed through gRPC outgoing metadata. However, this would have required +to be exposed through gRPC outgoing headers. However, this would have required defining language specific APIs, which would increase the complexity of this change. @@ -164,6 +154,6 @@ Will provide an implementation in Go. [A42]: A42-xds-ring-hash-lb-policy.md [envoy-ringhash]: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash -[metadata]: https://grpc.io/docs/what-is-grpc/core-concepts/#metadata +[header]: https://grpc.io/docs/guides/metadata/#headers [service-config]: https://github.com/grpc/grpc/blob/master/doc/service_config.md [LbEndpoint.Metadata]: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/endpoint/v3/endpoint_components.proto#envoy-v3-api-field-config-endpoint-v3-lbendpoint-metadata From cf65a146b3b4f9e34b2fd22aa9d7e2f2deb67e43 Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Sun, 25 Feb 2024 21:53:51 +0100 Subject: [PATCH 05/11] better handling of empty hash header --- A76-ring-hash-improvements.md | 46 ++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md index e1e73f8f6..5ba201679 100644 --- a/A76-ring-hash-improvements.md +++ b/A76-ring-hash-improvements.md @@ -93,13 +93,22 @@ At pick time: then the request hash key will be set to this value. If `request_hash_header` contains multiple values, then values are joined with a comma `,` character between each value before hashing. -- If `request_hash_header` is not empty, and the request has no value or an -empty value associated with the header defined, then the picker will generate a -random hash for it. The use of a random hash key will effectively sends the -request to a random endpoint. - If `request_hash_header` is empty, then the request hash key will be based on the xDS hash policy in RDS, which keeps the existing LB configuration for ring hash working as before with xDS. +- If `request_hash_header` is not empty, and the request has no value associated +with the header or its value is empty, then the picker behave as follows: the +picker will generate a random hash for the request. If the use of this random +hash triggers a connection attempt (according to the rules described in [A42: +Picker Behavior][A42-picker-behavior]), then before queuing the pick, the picker +will scan forward searching for a subchannel in `READY` state. If it finds a +subchannel in `READY` state, the picker returns it. + +This behavior ensures that a single request creates at most one new connection, +and that missing header does not add extra latency in the common case where +there is already at least one subchannel in `READY` state. It converges to +picking a random host, since each request may create a new connection to a +random endpoint. ### Explicitly setting the endpoint hash key @@ -111,11 +120,10 @@ not set or empty, then the endpoint IP address is hashed, matching the current behavior. The locations of an existing endpoint on the ring is updated if its `hash_key` endpoint attribute changes. -For xDS, the cluster resolver LB policy responsible for translating EDS -responses into resolver endpoints will be changed to set the `hash_key` 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-ring-hash]. +The xDS resolver, described in [A74][A74], will be changed to set the `hash_key` +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-ring-hash]. ### Temporary environment variable protection @@ -148,12 +156,32 @@ through name resolver attributes, instead of only extracting the specific `hash_key` attribute, so as to make them available to custom LB policies. We decided to keep only extract `hash_key` to limit the scope of this gRFC. +We discussed various option to handle requests that are missing a hash key in +the non-xDS case. When using ring hash with xDS, the hash is assigned a random +value in the xDS config selector, which ensure all picks for this request can +trigger a connection to at most one endpoint. However, without xDS, there is no +place in the code to assign the hash such that it retains this property. We +considered the following alternative solutions: +1. Add a config selector or filter to pick the hash. There currently is no + public API to do so from the service config, so we would have had to define + one. +2. Using an internal request attribute to set the hash. Again, there is no + cross-language public API for this. +3. Failing the pick. We generally prefer the lack of a header to affect load + balancing but not correctness, so this option was not ideal. +4. Treating a missing header as being present but having the empty string for + value. All instances of the channel would end up picking the same endpoint to + send requests with a missing header, which could overload this endpoint if a + lot of requests do not have a request hash key. + ## Implementation Will provide an implementation in Go. [A42]: A42-xds-ring-hash-lb-policy.md +[A74]: A74-xds-config-tears.md [envoy-ringhash]: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash [header]: https://grpc.io/docs/guides/metadata/#headers [service-config]: https://github.com/grpc/grpc/blob/master/doc/service_config.md [LbEndpoint.Metadata]: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/endpoint/v3/endpoint_components.proto#envoy-v3-api-field-config-endpoint-v3-lbendpoint-metadata +[A42-picker-behavior]: A42-xds-ring-hash-lb-policy.md#picker-behavior From 202a2253c43bf436ecb185f6f71064b058dbca23 Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Tue, 2 Apr 2024 12:58:20 +0200 Subject: [PATCH 06/11] feedback from Mark --- A76-ring-hash-improvements.md | 87 +++++++++++++++++++++++++++-------- 1 file changed, 67 insertions(+), 20 deletions(-) diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md index 5ba201679..5b9c9849a 100644 --- a/A76-ring-hash-improvements.md +++ b/A76-ring-hash-improvements.md @@ -4,7 +4,7 @@ A76: Improvements to the Ring Hash LB Policy * Approver: markdroth * Status: Draft * Implemented in: -* Last updated: 2024-01-23 +* Last updated: 2024-04-02 * Discussion at: https://groups.google.com/g/grpc-io/c/ZKI1RIF0e_s/m/oBXqOFb0AQAJ ## Abstract @@ -65,9 +65,11 @@ attribute through the language specific resolver attribute interface. ### Related Proposals: -This proposal extends the following existing gRFC: +This proposal extends the following existing gRFCs: * [gRFC A42: xDS Ring Hash LB Policy][A42] +* [gRFC A61: IPv4 and IPv6 Dualstack Backend Support][A61] +* [gRFC A74: xDS Config Tears][A74] ## Proposal @@ -89,26 +91,69 @@ rejected. If the `request_hash_header` refers to a binary header (suffixed with `-bin`), the configuration is also rejected. At pick time: -- If `request_hash_header` is not empty, and the header has a non-empty value, -then the request hash key will be set to this value. If `request_hash_header` -contains multiple values, then values are joined with a comma `,` character -between each value before hashing. - If `request_hash_header` is empty, then the request hash key will be based on the xDS hash policy in RDS, which keeps the existing LB configuration for ring hash working as before with xDS. +- If `request_hash_header` is not empty, and the header has a non-empty value, +then the request hash key will be set to this value. If the header contains +multiple values, then values are joined with a comma `,` character before +hashing. - If `request_hash_header` is not empty, and the request has no value associated -with the header or its value is empty, then the picker behave as follows: the -picker will generate a random hash for the request. If the use of this random -hash triggers a connection attempt (according to the rules described in [A42: -Picker Behavior][A42-picker-behavior]), then before queuing the pick, the picker -will scan forward searching for a subchannel in `READY` state. If it finds a -subchannel in `READY` state, the picker returns it. +with the header or its value is empty, then the picker will generate a random +hash for the request. If the use of this random hash triggers a connection +attempt (according to the rules described in [A42: Picker +Behavior][A42-picker-behavior] and updated in [A61: Ring Hash][A61-ring-hash]), +then before queuing the pick, the picker will scan forward searching for a +subchannel in `READY` state. If it finds a subchannel in `READY` state, the +picker returns it. + +The following pseudo code describes the updated picker logic: + +``` +// Determine request hash. +using_random_hash = false; +if (config.request_hash_header.empty()) { + request_hash = call_attributes.hash; +} else { + header = headers.find(config.request_hash_header); + if (header == null) { + using_random_hash = true; + request_hash = ComputeRandomHash(); + } else { + request_hash = ComputeHash(header); + } +} + +// Do pick based on hash. +first_index = ring.FindIndexForHash(request_hash); +requested_connection = false; +for (i = 0; i < ring.size(); ++i) { + index = (first_index + i) % ring.size(); + if (ring[index].state == READY) { + return ring[index].picker->Pick(...); + } + if (requested_connection) continue; + if (ring[index].state == IDLE) { + ring[index].endpoint.TriggerConnectionAttemptInControlPlane(); + if (using_random_hash) { + requested_connection = true; + continue; + } + return PICK_QUEUE; + } + if (ring[index].state == CONNECTING) { + return PICK_QUEUE; + } +} +if (requested_connection) return PICK_QUEUE; +return PICK_FAIL; +``` This behavior ensures that a single request creates at most one new connection, -and that missing header does not add extra latency in the common case where -there is already at least one subchannel in `READY` state. It converges to -picking a random host, since each request may create a new connection to a -random endpoint. +and that a request missing the header does not add extra latency in the common +case where there is already at least one subchannel in `READY` state. It +converges to picking a random host, since each request may create a new +connection to a random endpoint. ### Explicitly setting the endpoint hash key @@ -116,9 +161,9 @@ The `ring_hash` policy will be changed such that the hash key used for determining the locations of each endpoint on the ring will be extracted from a pre-defined endpoint attribute called `hash_key`. If this attribute is set, then the endpoint is placed on the ring by hashing its value. If this attribute is -not set or empty, then the endpoint IP address is hashed, matching the current -behavior. The locations of an existing endpoint on the ring is updated if its -`hash_key` endpoint attribute changes. +not set or empty, then the endpoint's first address is hashed, matching the +current behavior. The locations of an existing endpoint on the ring is updated +if its `hash_key` endpoint attribute changes. The xDS resolver, described in [A74][A74], will be changed to set the `hash_key` endpoint attribute to the value of [LbEndpoint.Metadata][LbEndpoint.Metadata] @@ -150,7 +195,7 @@ hash key. The advantage would have been that the request hash key would not have to be exposed through gRPC outgoing headers. However, this would have required defining language specific APIs, which would increase the complexity of this change. - + We also discussed the option of exposing all `LbEndpoint.metadata` from EDS through name resolver attributes, instead of only extracting the specific `hash_key` attribute, so as to make them available to custom LB policies. We @@ -179,9 +224,11 @@ considered the following alternative solutions: Will provide an implementation in Go. [A42]: A42-xds-ring-hash-lb-policy.md +[A61]: A61-IPv4-IPv6-dualstack-backends.md [A74]: A74-xds-config-tears.md [envoy-ringhash]: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/upstream/load_balancing/load_balancers#ring-hash [header]: https://grpc.io/docs/guides/metadata/#headers [service-config]: https://github.com/grpc/grpc/blob/master/doc/service_config.md [LbEndpoint.Metadata]: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/endpoint/v3/endpoint_components.proto#envoy-v3-api-field-config-endpoint-v3-lbendpoint-metadata [A42-picker-behavior]: A42-xds-ring-hash-lb-policy.md#picker-behavior +[A61-ring-hash]: A61-IPv4-IPv6-dualstack-backends.md#ring-hash From 0de42f2176d70f0cdff9aeef9830b9eec44d039d Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Fri, 26 Apr 2024 17:10:20 +0200 Subject: [PATCH 07/11] add implementation link --- A76-ring-hash-improvements.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md index 5b9c9849a..f80f4a047 100644 --- a/A76-ring-hash-improvements.md +++ b/A76-ring-hash-improvements.md @@ -3,8 +3,8 @@ A76: Improvements to the Ring Hash LB Policy * Author(s): atollena * Approver: markdroth * Status: Draft -* Implemented in: -* Last updated: 2024-04-02 +* Implemented in: Go +* Last updated: 2024-04-26 * Discussion at: https://groups.google.com/g/grpc-io/c/ZKI1RIF0e_s/m/oBXqOFb0AQAJ ## Abstract @@ -221,7 +221,9 @@ considered the following alternative solutions: ## Implementation -Will provide an implementation in Go. +Implemented in Go: +- Allow setting the request hash key: https://github.com/grpc/grpc-go/pull/7170 +- Make endpoint hash key configurable, and set it from EDS: https://github.com/grpc/grpc-go/pull/7161 [A42]: A42-xds-ring-hash-lb-policy.md [A61]: A61-IPv4-IPv6-dualstack-backends.md From 3458c0a1a30296a85633736123a97262368ef8c8 Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Mon, 22 Jul 2024 16:04:31 +0200 Subject: [PATCH 08/11] feedback from Eric --- A76-ring-hash-improvements.md | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md index f80f4a047..edd15ac5d 100644 --- a/A76-ring-hash-improvements.md +++ b/A76-ring-hash-improvements.md @@ -99,13 +99,12 @@ then the request hash key will be set to this value. If the header contains multiple values, then values are joined with a comma `,` character before hashing. - If `request_hash_header` is not empty, and the request has no value associated -with the header or its value is empty, then the picker will generate a random -hash for the request. If the use of this random hash triggers a connection -attempt (according to the rules described in [A42: Picker -Behavior][A42-picker-behavior] and updated in [A61: Ring Hash][A61-ring-hash]), -then before queuing the pick, the picker will scan forward searching for a -subchannel in `READY` state. If it finds a subchannel in `READY` state, the -picker returns it. +with the header, then the picker will generate a random hash for the request. If +the use of this random hash triggers a connection attempt (according to the +rules described in [A42: Picker Behavior][A42-picker-behavior] and updated in +[A61: Ring Hash][A61-ring-hash]), then before queuing the pick, the picker will +scan forward searching for a subchannel in `READY` state. If it finds a +subchannel in `READY` state, the picker returns it. The following pseudo code describes the updated picker logic: @@ -168,16 +167,16 @@ if its `hash_key` endpoint attribute changes. The xDS resolver, described in [A74][A74], will be changed to set the `hash_key` 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-ring-hash]. +hash load balancer][envoy-ringhash]. ### Temporary environment variable protection -Explicitly setting the request hash key cannot possibly lead to problem with -existing deployment because the new behavior requires setting a load balancing -policy configuration field that did not exist before. Therefore, it is not gated -behind an environment variable. +Explicitly setting the request hash key will be gated by the +`GRPC_EXPERIMENTAL_RING_HASH_SET_REQUEST_HASH_KEY` environment variable. This +mechanism will be supported for a couple of gRPC releases but will be removed in +the long run. -Adding support for the hash_key in xDS endpoint metadata could potentially break +Adding support for the `hash_key` in xDS endpoint metadata could potentially break existing clients whose control plane is setting this key, because upgrading the client to a new version of gRPC would automatically cause the key to start being used. We expect that this change will not cause problems for most users, but @@ -221,9 +220,7 @@ considered the following alternative solutions: ## Implementation -Implemented in Go: -- Allow setting the request hash key: https://github.com/grpc/grpc-go/pull/7170 -- Make endpoint hash key configurable, and set it from EDS: https://github.com/grpc/grpc-go/pull/7161 +Will be implemented in Go first. [A42]: A42-xds-ring-hash-lb-policy.md [A61]: A61-IPv4-IPv6-dualstack-backends.md From 41209331bb27429d64d922ec7fad090ae1ffc156 Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Thu, 25 Jul 2024 15:29:00 +0200 Subject: [PATCH 09/11] feedback from eric & mark --- A61-IPv4-IPv6-dualstack-backends.md | 3 ++- A76-ring-hash-improvements.md | 40 ++++++++++++----------------- 2 files changed, 19 insertions(+), 24 deletions(-) diff --git a/A61-IPv4-IPv6-dualstack-backends.md b/A61-IPv4-IPv6-dualstack-backends.md index 8080305e5..499f56b29 100644 --- a/A61-IPv4-IPv6-dualstack-backends.md +++ b/A61-IPv4-IPv6-dualstack-backends.md @@ -545,6 +545,7 @@ for (i = 0; i < ring.size(); ++i) { return PICK_QUEUE; } } +return PICK_FAIL; ``` As per [gRFC A42][A42], the ring_hash policy normally requires pick @@ -562,7 +563,7 @@ restored. However, with the sticky-TF behavior, it will not be possible to attempt to connect to only one endpoint at a time, because when a given pick_first child reports TRANSIENT_FAILURE, it will automatically try reconnecting after the backoff period without waiting for a connection -to be requested. Proposed psuedo-code for this logic is: +to be requested. Proposed pseudo-code for this logic is: ``` if (in_transient_failure && endpoint_entered_transient_failure) { diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md index edd15ac5d..ad43f435e 100644 --- a/A76-ring-hash-improvements.md +++ b/A76-ring-hash-improvements.md @@ -99,12 +99,10 @@ then the request hash key will be set to this value. If the header contains multiple values, then values are joined with a comma `,` character before hashing. - If `request_hash_header` is not empty, and the request has no value associated -with the header, then the picker will generate a random hash for the request. If -the use of this random hash triggers a connection attempt (according to the -rules described in [A42: Picker Behavior][A42-picker-behavior] and updated in -[A61: Ring Hash][A61-ring-hash]), then before queuing the pick, the picker will -scan forward searching for a subchannel in `READY` state. If it finds a -subchannel in `READY` state, the picker returns it. +with the header, then the picker will generate a random hash for the request. It +will walk the ring from this hash, and pick the first `READY` endpoint. If no +endpoint is currently in `CONNECTING` state, it will trigger a connection +attempt on at most one endpoint that is in `IDLE` state along the way. The following pseudo code describes the updated picker logic: @@ -123,36 +121,32 @@ if (config.request_hash_header.empty()) { } } -// Do pick based on hash. first_index = ring.FindIndexForHash(request_hash); -requested_connection = false; +if !using_random_hash { + // Use the logic from A62 unchanged. + // ... +} + +requested_connection = picker_has_a_child_connecting; for (i = 0; i < ring.size(); ++i) { index = (first_index + i) % ring.size(); if (ring[index].state == READY) { return ring[index].picker->Pick(...); } - if (requested_connection) continue; - if (ring[index].state == IDLE) { + if (!requested_connection && ring[index].state == IDLE) { ring[index].endpoint.TriggerConnectionAttemptInControlPlane(); - if (using_random_hash) { - requested_connection = true; - continue; - } - return PICK_QUEUE; - } - if (ring[index].state == CONNECTING) { - return PICK_QUEUE; + requested_connection = true; } } if (requested_connection) return PICK_QUEUE; return PICK_FAIL; ``` -This behavior ensures that a single request creates at most one new connection, -and that a request missing the header does not add extra latency in the common -case where there is already at least one subchannel in `READY` state. It -converges to picking a random host, since each request may create a new -connection to a random endpoint. +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`. ### Explicitly setting the endpoint hash key From 44ef7d3abe694f2616822c7c80ce8e2a58722e87 Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Thu, 25 Jul 2024 16:48:16 +0200 Subject: [PATCH 10/11] state out how we know that at least one endpoint is connecting --- A76-ring-hash-improvements.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md index ad43f435e..4b1490cb0 100644 --- a/A76-ring-hash-improvements.md +++ b/A76-ring-hash-improvements.md @@ -104,6 +104,11 @@ will walk the ring from this hash, and pick the first `READY` endpoint. If no endpoint is currently in `CONNECTING` state, it will trigger a connection attempt on at most one endpoint that is in `IDLE` state along the way. +When a new picker is created, we will compute whether at least one of the +endpoints is connecting, and store that information in the picker +(`picker_has_a_child_connecting` in the pseudo code below). This avoids having +to compute this information on every pick when using a random hash. + The following pseudo code describes the updated picker logic: ``` From 7edaaad59cdf94e4b3ea50a145c8a0398152a6b0 Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Fri, 26 Jul 2024 10:55:21 +0200 Subject: [PATCH 11/11] comments from doug + use first pick on TF --- A61-IPv4-IPv6-dualstack-backends.md | 3 +- A76-ring-hash-improvements.md | 53 +++++++++++++++++------------ 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/A61-IPv4-IPv6-dualstack-backends.md b/A61-IPv4-IPv6-dualstack-backends.md index 499f56b29..7e1623d44 100644 --- a/A61-IPv4-IPv6-dualstack-backends.md +++ b/A61-IPv4-IPv6-dualstack-backends.md @@ -545,7 +545,8 @@ for (i = 0; i < ring.size(); ++i) { return PICK_QUEUE; } } -return PICK_FAIL; +// All children are in transient failure. Return the first failure. +return ring[first_index].picker->Pick(...); ``` As per [gRFC A42][A42], the ring_hash policy normally requires pick diff --git a/A76-ring-hash-improvements.md b/A76-ring-hash-improvements.md index 4b1490cb0..6e6f81b6d 100644 --- a/A76-ring-hash-improvements.md +++ b/A76-ring-hash-improvements.md @@ -93,16 +93,19 @@ rejected. If the `request_hash_header` refers to a binary header (suffixed with At pick time: - If `request_hash_header` is empty, then the request hash key will be based on the xDS hash policy in RDS, which keeps the existing LB configuration for ring -hash working as before with xDS. +hash working as before with xDS. If the request hash key has not been set by +xDS, then we will generate a random hash for the request. - If `request_hash_header` is not empty, and the header has a non-empty value, then the request hash key will be set to this value. If the header contains multiple values, then values are joined with a comma `,` character before hashing. - If `request_hash_header` is not empty, and the request has no value associated -with the header, then the picker will generate a random hash for the request. It -will walk the ring from this hash, and pick the first `READY` endpoint. If no -endpoint is currently in `CONNECTING` state, it will trigger a connection -attempt on at most one endpoint that is in `IDLE` state along the way. +with the header, then the picker will generate a random hash for the request. + +If the picker has generated a random hash, it will walk the ring from this hash, +and pick the first `READY` endpoint. If no endpoint is currently in `CONNECTING` +state, it will trigger a connection attempt on at most one endpoint that is in +`IDLE` state along the way. When a new picker is created, we will compute whether at least one of the endpoints is connecting, and store that information in the picker @@ -115,7 +118,13 @@ The following pseudo code describes the updated picker logic: // Determine request hash. using_random_hash = false; if (config.request_hash_header.empty()) { - request_hash = call_attributes.hash; + if (call_attributes.hash.has_value()) { + // Set by the xDS config selector. + request_hash = call_attributes.hash; + } else { + using_random_hash = true; + request_hash = ComputeRandomHash(); + } } else { header = headers.find(config.request_hash_header); if (header == null) { @@ -128,7 +137,7 @@ if (config.request_hash_header.empty()) { first_index = ring.FindIndexForHash(request_hash); if !using_random_hash { - // Use the logic from A62 unchanged. + // Return based on A62 unchanged. // ... } @@ -144,7 +153,8 @@ for (i = 0; i < ring.size(); ++i) { } } if (requested_connection) return PICK_QUEUE; -return PICK_FAIL; +// All children are in transient failure. Return the first failure. +return ring[first_index].picker->Pick(...); ``` This behavior ensures that a single RPC does not cause more than one endpoint to @@ -171,20 +181,21 @@ hash load balancer][envoy-ringhash]. ### Temporary environment variable protection Explicitly setting the request hash key will be gated by the -`GRPC_EXPERIMENTAL_RING_HASH_SET_REQUEST_HASH_KEY` environment variable. This -mechanism will be supported for a couple of gRPC releases but will be removed in -the long run. - -Adding support for the `hash_key` in xDS endpoint metadata could potentially break -existing clients whose control plane is setting this key, because upgrading the -client to a new version of gRPC would automatically cause the key to start being -used. We expect that this change will not cause problems for most users, but -just in case there is a problem, we will provide a migration story by supporting -a temporary mechanism to tell gRPC to ignore the `hash_key` endpoint +`GRPC_EXPERIMENTAL_RING_HASH_SET_REQUEST_HASH_KEY` environment variable until +sufficiently tested. + +Adding support for the `hash_key` in xDS endpoint metadata could potentially +break existing clients whose control plane is setting this key, because +upgrading the client to a new version of gRPC would automatically cause the key +to start being used. We expect that this change will not cause problems for most +users, but just in case there is a problem, we will provide a migration story by +supporting a temporary mechanism to tell gRPC to ignore the `hash_key` endpoint metadata. This will be enabled by setting the -`GRPC_XDS_ENDPOINT_HASH_KEY_BACKWARD_COMPAT` environment variable to true. This -mechanism will be supported for a couple of gRPC releases but will be removed in -the long run. +`GRPC_XDS_ENDPOINT_HASH_KEY_BACKWARD_COMPAT` environment variable to `true`. The +first release will have the environment variable assume `true` if not set. A +subsequent release will set it to `false` if not set, enabling the new +behavior. A final release will remove the opt-out environment variable and leave +the new behavior enabled. ## Rationale