From 5de65a6d504ad9557572d04cf3f84e9caa9201fe Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Fri, 6 Sep 2024 11:43:07 -0700 Subject: [PATCH] use an attribute from resolved addresses IS_PETIOLE_POLICY to control whether or not health checking is supported (#11513) * use an attribute from resolved addresses IS_PETIOLE_POLICY to control whether or not health checking is supported so that top level versions can't do any health checking, while those under petiole policies can. Fixes #11413 --- .../internal/PickFirstLeafLoadBalancer.java | 17 +++++++-- .../PickFirstLeafLoadBalancerTest.java | 36 +++++++++++++++++-- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java b/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java index 344012fef2b..bfa462e16e1 100644 --- a/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java +++ b/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java @@ -69,6 +69,7 @@ final class PickFirstLeafLoadBalancer extends LoadBalancer { private ConnectivityState concludedState = IDLE; private final boolean enableHappyEyeballs = PickFirstLoadBalancerProvider.isEnabledHappyEyeballs(); + private boolean notAPetiolePolicy = true; // means not under a petiole policy PickFirstLeafLoadBalancer(Helper helper) { this.helper = checkNotNull(helper, "helper"); @@ -80,6 +81,10 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { return Status.FAILED_PRECONDITION.withDescription("Already shut down"); } + // Cache whether or not this is a petiole policy, which is based off of an address attribute + Boolean isPetiolePolicy = resolvedAddresses.getAttributes().get(IS_PETIOLE_POLICY); + this.notAPetiolePolicy = isPetiolePolicy == null || !isPetiolePolicy; + List servers = resolvedAddresses.getAddresses(); // Validate the address list @@ -303,7 +308,8 @@ private void updateHealthCheckedState(SubchannelData subchannelData) { if (subchannelData.state != READY) { return; } - if (subchannelData.getHealthState() == READY) { + + if (notAPetiolePolicy || subchannelData.getHealthState() == READY) { updateBalancingState(READY, new FixedResultPicker(PickResult.withSubchannel(subchannelData.subchannel))); } else if (subchannelData.getHealthState() == TRANSIENT_FAILURE) { @@ -444,7 +450,7 @@ private SubchannelData createNewSubchannel(SocketAddress addr, Attributes attrs) hcListener.subchannelData = subchannelData; subchannels.put(addr, subchannelData); Attributes scAttrs = subchannel.getAttributes(); - if (scAttrs.get(LoadBalancer.HAS_HEALTH_PRODUCER_LISTENER_KEY) == null) { + if (notAPetiolePolicy || scAttrs.get(LoadBalancer.HAS_HEALTH_PRODUCER_LISTENER_KEY) == null) { subchannelData.healthStateInfo = ConnectivityStateInfo.forNonError(READY); } subchannel.start(stateInfo -> processSubchannelState(subchannelData, stateInfo)); @@ -468,6 +474,13 @@ private final class HealthListener implements SubchannelStateListener { @Override public void onSubchannelState(ConnectivityStateInfo newState) { + if (notAPetiolePolicy) { + log.log(Level.WARNING, + "Ignoring health status {0} for subchannel {1} as this is not under a petiole policy", + new Object[]{newState, subchannelData.subchannel}); + return; + } + log.log(Level.FINE, "Received health status {0} for subchannel {1}", new Object[]{newState, subchannelData.subchannel}); subchannelData.healthStateInfo = newState; diff --git a/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java b/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java index 9c22c4a7086..63915bddc99 100644 --- a/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java +++ b/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java @@ -25,6 +25,7 @@ import static io.grpc.ConnectivityState.TRANSIENT_FAILURE; import static io.grpc.LoadBalancer.HAS_HEALTH_PRODUCER_LISTENER_KEY; import static io.grpc.LoadBalancer.HEALTH_CONSUMER_LISTENER_ARG_KEY; +import static io.grpc.LoadBalancer.IS_PETIOLE_POLICY; import static io.grpc.internal.PickFirstLeafLoadBalancer.CONNECTION_DELAY_INTERVAL_MS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -389,15 +390,44 @@ public void pickAfterResolvedAndChanged() { verify(mockSubchannel2).requestConnection(); } + @Test + public void healthCheck_nonPetiolePolicy() { + when(mockSubchannel1.getAttributes()).thenReturn( + Attributes.newBuilder().set(HAS_HEALTH_PRODUCER_LISTENER_KEY, true).build()); + + // Initialize with one server loadbalancer and both health and state listeners + List oneServer = Lists.newArrayList(servers.get(0)); + loadBalancer.acceptResolvedAddresses(ResolvedAddresses.newBuilder().setAddresses(oneServer) + .setAttributes(Attributes.EMPTY).build()); + InOrder inOrder = inOrder(mockHelper, mockSubchannel1); + inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), any(SubchannelPicker.class)); + inOrder.verify(mockHelper).createSubchannel(createArgsCaptor.capture()); + SubchannelStateListener healthListener = createArgsCaptor.getValue() + .getOption(HEALTH_CONSUMER_LISTENER_ARG_KEY); + inOrder.verify(mockSubchannel1).start(stateListenerCaptor.capture()); + SubchannelStateListener stateListener = stateListenerCaptor.getValue(); + + stateListener.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING)); + healthListener.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING)); + inOrder.verify(mockHelper, never()).updateBalancingState(any(), any()); + + stateListener.onSubchannelState(ConnectivityStateInfo.forNonError(READY)); + inOrder.verify(mockHelper).updateBalancingState(eq(READY), any()); // health listener ignored + + healthListener.onSubchannelState(ConnectivityStateInfo.forTransientFailure(Status.INTERNAL)); + inOrder.verify(mockHelper, never()).updateBalancingState(any(), any(SubchannelPicker.class)); + } + @Test public void healthCheckFlow() { when(mockSubchannel1.getAttributes()).thenReturn( Attributes.newBuilder().set(HAS_HEALTH_PRODUCER_LISTENER_KEY, true).build()); when(mockSubchannel2.getAttributes()).thenReturn( Attributes.newBuilder().set(HAS_HEALTH_PRODUCER_LISTENER_KEY, true).build()); + List oneServer = Lists.newArrayList(servers.get(0), servers.get(1)); loadBalancer.acceptResolvedAddresses(ResolvedAddresses.newBuilder().setAddresses(oneServer) - .setAttributes(Attributes.EMPTY).build()); + .setAttributes(Attributes.newBuilder().set(IS_PETIOLE_POLICY, true).build()).build()); InOrder inOrder = inOrder(mockHelper, mockSubchannel1, mockSubchannel2); inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), any(SubchannelPicker.class)); @@ -413,13 +443,13 @@ public void healthCheckFlow() { // subchannel2 | IDLE | IDLE stateListener.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING)); healthListener.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING)); - inOrder.verify(mockHelper, times(0)).updateBalancingState(any(), any()); + inOrder.verify(mockHelper, never()).updateBalancingState(any(), any()); // subchannel | state | health // subchannel1 | READY | CONNECTING // subchannel2 | IDLE | IDLE stateListener.onSubchannelState(ConnectivityStateInfo.forNonError(READY)); - inOrder.verify(mockHelper, times(0)).updateBalancingState(any(), any()); + inOrder.verify(mockHelper, never()).updateBalancingState(any(), any()); // subchannel | state | health // subchannel1 | READY | READY