From 36d1d5fe457eb102043f39a5664d020c28c2a024 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 12 May 2022 12:51:21 -0700 Subject: [PATCH] xsd: wrr_locality to not propagate locality weight attribute (#9158) WrrLocalityLoadBalancer should remove the locality weights attribute from Resolved addresses after using the information. Not propagating this attribute will make it impossible for another child wrr_locality from working. This is not a supported situation and this change make the failure happen earlier and to be more obvious as to the cause. --- .../io/grpc/xds/WrrLocalityLoadBalancer.java | 8 ++++++++ .../grpc/xds/WrrLocalityLoadBalancerTest.java | 17 ++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/xds/src/main/java/io/grpc/xds/WrrLocalityLoadBalancer.java b/xds/src/main/java/io/grpc/xds/WrrLocalityLoadBalancer.java index 097aa23fd6..61c6b103d8 100644 --- a/xds/src/main/java/io/grpc/xds/WrrLocalityLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/WrrLocalityLoadBalancer.java @@ -80,6 +80,14 @@ final class WrrLocalityLoadBalancer extends LoadBalancer { wrrLocalityConfig.childPolicy)); } + // Remove the locality weights attribute now that we have consumed it. This is done simply for + // ease of debugging for the unsupported (and unlikely) scenario where WrrLocalityConfig has + // another wrr_locality as the child policy. The missing locality weight attribute would make + // the child wrr_locality fail early. + resolvedAddresses = resolvedAddresses.toBuilder() + .setAttributes(resolvedAddresses.getAttributes().toBuilder() + .discard(InternalXdsAttributes.ATTR_LOCALITY_WEIGHTS).build()).build(); + switchLb.switchTo(wrrLocalityConfig.childPolicy.getProvider()); switchLb.handleResolvedAddresses( resolvedAddresses.toBuilder() diff --git a/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java index d8c1ee599e..f1474fcf15 100644 --- a/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java @@ -112,7 +112,6 @@ public class WrrLocalityLoadBalancerTest { new WeightedPolicySelection(1, childPolicy)); assertThat(wtConfig.targets).containsEntry(localityTwo.toString(), new WeightedPolicySelection(2, childPolicy)); - } @Test @@ -154,6 +153,22 @@ public class WrrLocalityLoadBalancerTest { verify(mockChildLb).handleNameResolutionError(Status.DEADLINE_EXCEEDED); } + @Test + public void localityWeightAttributeNotPropagated() { + Locality locality = Locality.create("region1", "zone1", "subzone1"); + PolicySelection childPolicy = new PolicySelection(mockProvider, null); + + WrrLocalityConfig wlConfig = new WrrLocalityConfig(childPolicy); + Map localityWeights = ImmutableMap.of(locality, 1); + deliverAddresses(wlConfig, localityWeights); + + // Assert that the child policy and the locality weights were correctly mapped to a + // WeightedTargetConfig. + verify(mockChildLb).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + assertThat(resolvedAddressesCaptor.getValue().getAttributes() + .get(InternalXdsAttributes.ATTR_LOCALITY_WEIGHTS)).isNull(); + } + @Test public void shutdown() { deliverAddresses(new WrrLocalityConfig(new PolicySelection(mockProvider, null)),