Skip to content

Commit

Permalink
core: throw away subchannel references after round_robin is shutdown (g…
Browse files Browse the repository at this point in the history
…rpc#8132)

Triggering balancing state updates after already being shutdown can be confusing for the upstream of round_robin. In cases of the callers not managing round_robin's lifecycle (e.g., not ignoring updates after it shuts down round_robin, which it should), it can make problem very bad, especially with the behavior that round_robin is actually propagating TRANSIENT_FAILURE with a picker that buffers RPCs.

This change only polishes round_robin by always preserving its invariant. Callers/LBs should not rely on this and should still manage the balancing updates from its downstream correctly based on the downstream's lifetime.
  • Loading branch information
voidzcy committed May 10, 2021
1 parent d22f93e commit dc24c19
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 0 deletions.
Expand Up @@ -163,6 +163,7 @@ public void shutdown() {
for (Subchannel subchannel : getSubchannels()) {
shutdownSubchannel(subchannel);
}
subchannels.clear();
}

private static final Status EMPTY_OK = Status.OK.withDescription("no subchannels ready");
Expand Down
19 changes: 19 additions & 0 deletions core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java
Expand Up @@ -290,6 +290,25 @@ public void pickAfterStateChange() throws Exception {
verifyNoMoreInteractions(mockHelper);
}

@Test
public void ignoreShutdownSubchannelStateChange() {
InOrder inOrder = inOrder(mockHelper);
loadBalancer.handleResolvedAddresses(
ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY)
.build());
inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class));

loadBalancer.shutdown();
for (Subchannel sc : loadBalancer.getSubchannels()) {
verify(sc).shutdown();
// When the subchannel is being shut down, a SHUTDOWN connectivity state is delivered
// back to the subchannel state listener.
deliverSubchannelState(sc, ConnectivityStateInfo.forNonError(SHUTDOWN));
}

inOrder.verifyNoMoreInteractions();
}

@Test
public void stayTransientFailureUntilReady() {
InOrder inOrder = inOrder(mockHelper);
Expand Down

0 comments on commit dc24c19

Please sign in to comment.