Skip to content

Commit 5de65a6

Browse files
larry-safranlarry-safran
authored
use an attribute from resolved addresses IS_PETIOLE_POLICY to control whether or not health checking is supported (grpc#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 grpc#11413
1 parent f6d2f20 commit 5de65a6

File tree

2 files changed

+48
-5
lines changed

2 files changed

+48
-5
lines changed

‎core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ final class PickFirstLeafLoadBalancer extends LoadBalancer {
6969
private ConnectivityState concludedState = IDLE;
7070
private final boolean enableHappyEyeballs =
7171
PickFirstLoadBalancerProvider.isEnabledHappyEyeballs();
72+
private boolean notAPetiolePolicy = true; // means not under a petiole policy
7273

7374
PickFirstLeafLoadBalancer(Helper helper) {
7475
this.helper = checkNotNull(helper, "helper");
@@ -80,6 +81,10 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
8081
return Status.FAILED_PRECONDITION.withDescription("Already shut down");
8182
}
8283

84+
// Cache whether or not this is a petiole policy, which is based off of an address attribute
85+
Boolean isPetiolePolicy = resolvedAddresses.getAttributes().get(IS_PETIOLE_POLICY);
86+
this.notAPetiolePolicy = isPetiolePolicy == null || !isPetiolePolicy;
87+
8388
List<EquivalentAddressGroup> servers = resolvedAddresses.getAddresses();
8489

8590
// Validate the address list
@@ -303,7 +308,8 @@ private void updateHealthCheckedState(SubchannelData subchannelData) {
303308
if (subchannelData.state != READY) {
304309
return;
305310
}
306-
if (subchannelData.getHealthState() == READY) {
311+
312+
if (notAPetiolePolicy || subchannelData.getHealthState() == READY) {
307313
updateBalancingState(READY,
308314
new FixedResultPicker(PickResult.withSubchannel(subchannelData.subchannel)));
309315
} else if (subchannelData.getHealthState() == TRANSIENT_FAILURE) {
@@ -444,7 +450,7 @@ private SubchannelData createNewSubchannel(SocketAddress addr, Attributes attrs)
444450
hcListener.subchannelData = subchannelData;
445451
subchannels.put(addr, subchannelData);
446452
Attributes scAttrs = subchannel.getAttributes();
447-
if (scAttrs.get(LoadBalancer.HAS_HEALTH_PRODUCER_LISTENER_KEY) == null) {
453+
if (notAPetiolePolicy || scAttrs.get(LoadBalancer.HAS_HEALTH_PRODUCER_LISTENER_KEY) == null) {
448454
subchannelData.healthStateInfo = ConnectivityStateInfo.forNonError(READY);
449455
}
450456
subchannel.start(stateInfo -> processSubchannelState(subchannelData, stateInfo));
@@ -468,6 +474,13 @@ private final class HealthListener implements SubchannelStateListener {
468474

469475
@Override
470476
public void onSubchannelState(ConnectivityStateInfo newState) {
477+
if (notAPetiolePolicy) {
478+
log.log(Level.WARNING,
479+
"Ignoring health status {0} for subchannel {1} as this is not under a petiole policy",
480+
new Object[]{newState, subchannelData.subchannel});
481+
return;
482+
}
483+
471484
log.log(Level.FINE, "Received health status {0} for subchannel {1}",
472485
new Object[]{newState, subchannelData.subchannel});
473486
subchannelData.healthStateInfo = newState;

‎core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
2626
import static io.grpc.LoadBalancer.HAS_HEALTH_PRODUCER_LISTENER_KEY;
2727
import static io.grpc.LoadBalancer.HEALTH_CONSUMER_LISTENER_ARG_KEY;
28+
import static io.grpc.LoadBalancer.IS_PETIOLE_POLICY;
2829
import static io.grpc.internal.PickFirstLeafLoadBalancer.CONNECTION_DELAY_INTERVAL_MS;
2930
import static org.junit.Assert.assertEquals;
3031
import static org.junit.Assert.assertFalse;
@@ -389,15 +390,44 @@ public void pickAfterResolvedAndChanged() {
389390
verify(mockSubchannel2).requestConnection();
390391
}
391392

393+
@Test
394+
public void healthCheck_nonPetiolePolicy() {
395+
when(mockSubchannel1.getAttributes()).thenReturn(
396+
Attributes.newBuilder().set(HAS_HEALTH_PRODUCER_LISTENER_KEY, true).build());
397+
398+
// Initialize with one server loadbalancer and both health and state listeners
399+
List<EquivalentAddressGroup> oneServer = Lists.newArrayList(servers.get(0));
400+
loadBalancer.acceptResolvedAddresses(ResolvedAddresses.newBuilder().setAddresses(oneServer)
401+
.setAttributes(Attributes.EMPTY).build());
402+
InOrder inOrder = inOrder(mockHelper, mockSubchannel1);
403+
inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), any(SubchannelPicker.class));
404+
inOrder.verify(mockHelper).createSubchannel(createArgsCaptor.capture());
405+
SubchannelStateListener healthListener = createArgsCaptor.getValue()
406+
.getOption(HEALTH_CONSUMER_LISTENER_ARG_KEY);
407+
inOrder.verify(mockSubchannel1).start(stateListenerCaptor.capture());
408+
SubchannelStateListener stateListener = stateListenerCaptor.getValue();
409+
410+
stateListener.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING));
411+
healthListener.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING));
412+
inOrder.verify(mockHelper, never()).updateBalancingState(any(), any());
413+
414+
stateListener.onSubchannelState(ConnectivityStateInfo.forNonError(READY));
415+
inOrder.verify(mockHelper).updateBalancingState(eq(READY), any()); // health listener ignored
416+
417+
healthListener.onSubchannelState(ConnectivityStateInfo.forTransientFailure(Status.INTERNAL));
418+
inOrder.verify(mockHelper, never()).updateBalancingState(any(), any(SubchannelPicker.class));
419+
}
420+
392421
@Test
393422
public void healthCheckFlow() {
394423
when(mockSubchannel1.getAttributes()).thenReturn(
395424
Attributes.newBuilder().set(HAS_HEALTH_PRODUCER_LISTENER_KEY, true).build());
396425
when(mockSubchannel2.getAttributes()).thenReturn(
397426
Attributes.newBuilder().set(HAS_HEALTH_PRODUCER_LISTENER_KEY, true).build());
427+
398428
List<EquivalentAddressGroup> oneServer = Lists.newArrayList(servers.get(0), servers.get(1));
399429
loadBalancer.acceptResolvedAddresses(ResolvedAddresses.newBuilder().setAddresses(oneServer)
400-
.setAttributes(Attributes.EMPTY).build());
430+
.setAttributes(Attributes.newBuilder().set(IS_PETIOLE_POLICY, true).build()).build());
401431

402432
InOrder inOrder = inOrder(mockHelper, mockSubchannel1, mockSubchannel2);
403433
inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), any(SubchannelPicker.class));
@@ -413,13 +443,13 @@ public void healthCheckFlow() {
413443
// subchannel2 | IDLE | IDLE
414444
stateListener.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING));
415445
healthListener.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING));
416-
inOrder.verify(mockHelper, times(0)).updateBalancingState(any(), any());
446+
inOrder.verify(mockHelper, never()).updateBalancingState(any(), any());
417447

418448
// subchannel | state | health
419449
// subchannel1 | READY | CONNECTING
420450
// subchannel2 | IDLE | IDLE
421451
stateListener.onSubchannelState(ConnectivityStateInfo.forNonError(READY));
422-
inOrder.verify(mockHelper, times(0)).updateBalancingState(any(), any());
452+
inOrder.verify(mockHelper, never()).updateBalancingState(any(), any());
423453

424454
// subchannel | state | health
425455
// subchannel1 | READY | READY

0 commit comments

Comments
 (0)