From faab2bed1979c15e70d7fed252eabd5986568cf9 Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Wed, 13 May 2026 11:56:07 +0530 Subject: [PATCH 1/3] xds: Disable Priority LB child policy retention cache --- .../io/grpc/xds/PriorityLoadBalancer.java | 10 +- .../io/grpc/xds/PriorityLoadBalancerTest.java | 179 ++++++++++++++---- 2 files changed, 150 insertions(+), 39 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java b/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java index 6e4566de76d..52fde6acb6d 100644 --- a/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java @@ -28,6 +28,7 @@ import io.grpc.Status; import io.grpc.SynchronizationContext; import io.grpc.SynchronizationContext.ScheduledHandle; +import io.grpc.internal.GrpcUtil; import io.grpc.util.ForwardingLoadBalancerHelper; import io.grpc.util.GracefulSwitchLoadBalancer; import io.grpc.xds.PriorityLoadBalancerProvider.PriorityLbConfig; @@ -73,6 +74,8 @@ final class PriorityLoadBalancer extends LoadBalancer { private SubchannelPicker currentPicker; // Set to true if currently in the process of handling resolved addresses. private boolean handlingResolvedAddresses; + private boolean isPriorityLbChildPolicyCacheEnabled = + GrpcUtil.getFlag("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE", false); PriorityLoadBalancer(Helper helper) { this.helper = checkNotNull(helper, "helper"); @@ -98,7 +101,12 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { if (!prioritySet.contains(priority)) { ChildLbState childLbState = children.get(priority); if (childLbState != null) { - childLbState.deactivate(); + if (isPriorityLbChildPolicyCacheEnabled) { + childLbState.deactivate(); + } else { + children.remove(priority); + childLbState.tearDown(); + } } } } diff --git a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java index beb568be9ce..21b1b12e147 100644 --- a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java @@ -149,6 +149,106 @@ public void tearDown() { @Test public void acceptResolvedAddresses() { + System.setProperty("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE", "true"); + try { + priorityLb = new PriorityLoadBalancer(helper); + SocketAddress socketAddress = new InetSocketAddress(8080); + EquivalentAddressGroup eag = new EquivalentAddressGroup(socketAddress); + eag = AddressFilter.setPathFilter(eag, ImmutableList.of("p1")); + List addresses = ImmutableList.of(eag); + Attributes attributes = + Attributes.newBuilder().set(Attributes.Key.create("fakeKey"), "fakeValue").build(); + Object fooConfig0 = new Object(); + PriorityChildConfig priorityChildConfig0 = + new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig0), true); + Object barConfig0 = new Object(); + PriorityChildConfig priorityChildConfig1 = + new PriorityChildConfig(newChildConfig(barLbProvider, barConfig0), true); + Object fooConfig1 = new Object(); + PriorityChildConfig priorityChildConfig2 = + new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig1), true); + PriorityLbConfig priorityLbConfig = + new PriorityLbConfig( + ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1, + "p2", priorityChildConfig2), + ImmutableList.of("p0", "p1", "p2")); + Status status = priorityLb.acceptResolvedAddresses( + ResolvedAddresses.newBuilder() + .setAddresses(addresses) + .setAttributes(attributes) + .setLoadBalancingPolicyConfig(priorityLbConfig) + .build()); + assertThat(status.getCode()).isEqualTo(Status.Code.OK); + assertThat(fooBalancers).hasSize(1); + assertThat(barBalancers).isEmpty(); + LoadBalancer fooBalancer0 = Iterables.getOnlyElement(fooBalancers); + verify(fooBalancer0).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + ResolvedAddresses addressesReceived = resolvedAddressesCaptor.getValue(); + assertThat(addressesReceived.getAddresses()).isEmpty(); + assertThat(addressesReceived.getAttributes()).isEqualTo(attributes); + assertThat(addressesReceived.getLoadBalancingPolicyConfig()).isEqualTo(fooConfig0); + + // Fail over to p1. + fakeClock.forwardTime(10, TimeUnit.SECONDS); + assertThat(fooBalancers).hasSize(1); + assertThat(barBalancers).hasSize(1); + LoadBalancer barBalancer0 = Iterables.getOnlyElement(barBalancers); + verify(barBalancer0).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + addressesReceived = resolvedAddressesCaptor.getValue(); + assertThat(Iterables.getOnlyElement(addressesReceived.getAddresses()).getAddresses()) + .containsExactly(socketAddress); + assertThat(addressesReceived.getAttributes()).isEqualTo(attributes); + assertThat(addressesReceived.getLoadBalancingPolicyConfig()).isEqualTo(barConfig0); + + // Fail over to p2. + fakeClock.forwardTime(10, TimeUnit.SECONDS); + assertThat(fooBalancers).hasSize(2); + assertThat(barBalancers).hasSize(1); + LoadBalancer fooBalancer1 = Iterables.getLast(fooBalancers); + verify(fooBalancer1).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + addressesReceived = resolvedAddressesCaptor.getValue(); + assertThat(addressesReceived.getAddresses()).isEmpty(); + assertThat(addressesReceived.getAttributes()).isEqualTo(attributes); + assertThat(addressesReceived.getLoadBalancingPolicyConfig()).isEqualTo(fooConfig1); + + // New update: p0 and p2 deleted; p1 config changed. + SocketAddress newSocketAddress = new InetSocketAddress(8081); + EquivalentAddressGroup newEag = new EquivalentAddressGroup(newSocketAddress); + newEag = AddressFilter.setPathFilter(newEag, ImmutableList.of("p1")); + List newAddresses = ImmutableList.of(newEag); + Object newBarConfig = new Object(); + PriorityLbConfig newPriorityLbConfig = + new PriorityLbConfig( + ImmutableMap.of("p1", + new PriorityChildConfig(newChildConfig(barLbProvider, newBarConfig), true)), + ImmutableList.of("p1")); + status = priorityLb.acceptResolvedAddresses( + ResolvedAddresses.newBuilder() + .setAddresses(newAddresses) + .setLoadBalancingPolicyConfig(newPriorityLbConfig) + .build()); + assertThat(status.getCode()).isEqualTo(Status.Code.OK); + assertThat(fooBalancers).hasSize(2); + assertThat(barBalancers).hasSize(1); + verify(barBalancer0, times(2)).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); + addressesReceived = resolvedAddressesCaptor.getValue(); + assertThat(Iterables.getOnlyElement(addressesReceived.getAddresses()).getAddresses()) + .containsExactly(newSocketAddress); + assertThat(addressesReceived.getAttributes()).isEqualTo(Attributes.EMPTY); + assertThat(addressesReceived.getLoadBalancingPolicyConfig()).isEqualTo(newBarConfig); + verify(fooBalancer0, never()).shutdown(); + verify(fooBalancer1, never()).shutdown(); + fakeClock.forwardTime(15, TimeUnit.MINUTES); + verify(fooBalancer0).shutdown(); + verify(fooBalancer1).shutdown(); + verify(barBalancer0, never()).shutdown(); + } finally { + System.clearProperty("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE"); + } + } + + @Test + public void acceptResolvedAddresses_cacheDisabled() { SocketAddress socketAddress = new InetSocketAddress(8080); EquivalentAddressGroup eag = new EquivalentAddressGroup(socketAddress); eag = AddressFilter.setPathFilter(eag, ImmutableList.of("p1")); @@ -233,9 +333,6 @@ public void acceptResolvedAddresses() { .containsExactly(newSocketAddress); assertThat(addressesReceived.getAttributes()).isEqualTo(Attributes.EMPTY); assertThat(addressesReceived.getLoadBalancingPolicyConfig()).isEqualTo(newBarConfig); - verify(fooBalancer0, never()).shutdown(); - verify(fooBalancer1, never()).shutdown(); - fakeClock.forwardTime(15, TimeUnit.MINUTES); verify(fooBalancer0).shutdown(); verify(fooBalancer1).shutdown(); verify(barBalancer0, never()).shutdown(); @@ -297,41 +394,47 @@ public void acceptResolvedAddresses_propagatesChildFailures() { @Test public void handleNameResolutionError() { - Object fooConfig0 = new Object(); - PriorityChildConfig priorityChildConfig0 = - new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig0), true); - Object fooConfig1 = new Object(); - PriorityChildConfig priorityChildConfig1 = - new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig1), true); - - PriorityLbConfig priorityLbConfig = - new PriorityLbConfig(ImmutableMap.of("p0", priorityChildConfig0), ImmutableList.of("p0")); - priorityLb.acceptResolvedAddresses( - ResolvedAddresses.newBuilder() - .setAddresses(ImmutableList.of()) - .setLoadBalancingPolicyConfig(priorityLbConfig) - .build()); - LoadBalancer fooLb0 = Iterables.getOnlyElement(fooBalancers); - Status status = Status.DATA_LOSS.withDescription("fake error"); - priorityLb.handleNameResolutionError(status); - verify(fooLb0).handleNameResolutionError(status); - - priorityLbConfig = - new PriorityLbConfig(ImmutableMap.of("p1", priorityChildConfig1), ImmutableList.of("p1")); - priorityLb.acceptResolvedAddresses( - ResolvedAddresses.newBuilder() - .setAddresses(ImmutableList.of()) - .setLoadBalancingPolicyConfig(priorityLbConfig) - .build()); - assertThat(fooBalancers).hasSize(2); - LoadBalancer fooLb1 = Iterables.getLast(fooBalancers); - status = Status.UNAVAILABLE.withDescription("fake error"); - priorityLb.handleNameResolutionError(status); - // fooLb0 is deactivated but not yet deleted. However, because it is delisted by the latest - // address update, name resolution error will not be propagated to it. - verify(fooLb0, never()).shutdown(); - verify(fooLb0, never()).handleNameResolutionError(status); - verify(fooLb1).handleNameResolutionError(status); + System.setProperty("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE", "true"); + try { + priorityLb = new PriorityLoadBalancer(helper); + Object fooConfig0 = new Object(); + PriorityChildConfig priorityChildConfig0 = + new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig0), true); + Object fooConfig1 = new Object(); + PriorityChildConfig priorityChildConfig1 = + new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig1), true); + + PriorityLbConfig priorityLbConfig = + new PriorityLbConfig(ImmutableMap.of("p0", priorityChildConfig0), ImmutableList.of("p0")); + priorityLb.acceptResolvedAddresses( + ResolvedAddresses.newBuilder() + .setAddresses(ImmutableList.of()) + .setLoadBalancingPolicyConfig(priorityLbConfig) + .build()); + LoadBalancer fooLb0 = Iterables.getOnlyElement(fooBalancers); + Status status = Status.DATA_LOSS.withDescription("fake error"); + priorityLb.handleNameResolutionError(status); + verify(fooLb0).handleNameResolutionError(status); + + priorityLbConfig = + new PriorityLbConfig(ImmutableMap.of("p1", priorityChildConfig1), ImmutableList.of("p1")); + priorityLb.acceptResolvedAddresses( + ResolvedAddresses.newBuilder() + .setAddresses(ImmutableList.of()) + .setLoadBalancingPolicyConfig(priorityLbConfig) + .build()); + assertThat(fooBalancers).hasSize(2); + LoadBalancer fooLb1 = Iterables.getLast(fooBalancers); + status = Status.UNAVAILABLE.withDescription("fake error"); + priorityLb.handleNameResolutionError(status); + // fooLb0 is deactivated but not yet deleted. However, because it is delisted by the latest + // address update, name resolution error will not be propagated to it. + verify(fooLb0, never()).shutdown(); + verify(fooLb0, never()).handleNameResolutionError(status); + verify(fooLb1).handleNameResolutionError(status); + } finally { + System.clearProperty("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE"); + } } @Test From 0ca6c99e77cc8222436c06536b865638f83ed30a Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Thu, 14 May 2026 00:18:27 +0530 Subject: [PATCH 2/3] resolve comments --- .../main/java/io/grpc/xds/PriorityLoadBalancer.java | 4 ++-- .../java/io/grpc/xds/PriorityLoadBalancerTest.java | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java b/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java index 52fde6acb6d..12efc05aeb6 100644 --- a/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java @@ -74,7 +74,7 @@ final class PriorityLoadBalancer extends LoadBalancer { private SubchannelPicker currentPicker; // Set to true if currently in the process of handling resolved addresses. private boolean handlingResolvedAddresses; - private boolean isPriorityLbChildPolicyCacheEnabled = + static boolean enablePriorityLbChildPolicyCache = GrpcUtil.getFlag("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE", false); PriorityLoadBalancer(Helper helper) { @@ -101,7 +101,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { if (!prioritySet.contains(priority)) { ChildLbState childLbState = children.get(priority); if (childLbState != null) { - if (isPriorityLbChildPolicyCacheEnabled) { + if (enablePriorityLbChildPolicyCache) { childLbState.deactivate(); } else { children.remove(priority); diff --git a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java index 21b1b12e147..988bc720e45 100644 --- a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java @@ -149,9 +149,9 @@ public void tearDown() { @Test public void acceptResolvedAddresses() { - System.setProperty("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE", "true"); + boolean originalFlagVal = PriorityLoadBalancer.enablePriorityLbChildPolicyCache; + PriorityLoadBalancer.enablePriorityLbChildPolicyCache = true; try { - priorityLb = new PriorityLoadBalancer(helper); SocketAddress socketAddress = new InetSocketAddress(8080); EquivalentAddressGroup eag = new EquivalentAddressGroup(socketAddress); eag = AddressFilter.setPathFilter(eag, ImmutableList.of("p1")); @@ -243,7 +243,7 @@ public void acceptResolvedAddresses() { verify(fooBalancer1).shutdown(); verify(barBalancer0, never()).shutdown(); } finally { - System.clearProperty("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE"); + PriorityLoadBalancer.enablePriorityLbChildPolicyCache = originalFlagVal; } } @@ -394,9 +394,9 @@ public void acceptResolvedAddresses_propagatesChildFailures() { @Test public void handleNameResolutionError() { - System.setProperty("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE", "true"); + boolean originalFlagVal = PriorityLoadBalancer.enablePriorityLbChildPolicyCache; + PriorityLoadBalancer.enablePriorityLbChildPolicyCache = true; try { - priorityLb = new PriorityLoadBalancer(helper); Object fooConfig0 = new Object(); PriorityChildConfig priorityChildConfig0 = new PriorityChildConfig(newChildConfig(fooLbProvider, fooConfig0), true); @@ -433,7 +433,7 @@ public void handleNameResolutionError() { verify(fooLb0, never()).handleNameResolutionError(status); verify(fooLb1).handleNameResolutionError(status); } finally { - System.clearProperty("GRPC_EXPERIMENTAL_ENABLE_PRIORITY_LB_CHILD_POLICY_CACHE"); + PriorityLoadBalancer.enablePriorityLbChildPolicyCache = originalFlagVal; } } From 6787fb8f73798de74f43a9355e18326ee6ae224e Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Thu, 14 May 2026 10:51:46 +0530 Subject: [PATCH 3/3] refactor lines --- xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java b/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java index 12efc05aeb6..ca142af0af3 100644 --- a/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java @@ -104,8 +104,8 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { if (enablePriorityLbChildPolicyCache) { childLbState.deactivate(); } else { - children.remove(priority); childLbState.tearDown(); + children.remove(priority); } } }