diff --git a/grpclb/src/main/java/io/grpc/grpclb/GrpclbState.java b/grpclb/src/main/java/io/grpc/grpclb/GrpclbState.java index 744d9620321..1c812e14500 100644 --- a/grpclb/src/main/java/io/grpc/grpclb/GrpclbState.java +++ b/grpclb/src/main/java/io/grpc/grpclb/GrpclbState.java @@ -94,6 +94,9 @@ final class GrpclbState { @VisibleForTesting static final Status NO_AVAILABLE_BACKENDS_STATUS = Status.UNAVAILABLE.withDescription("LoadBalancer responded without any backends"); + @VisibleForTesting + static final Status NO_FALLBACK_BACKENDS_FOUND_STATUS = + Status.UNAVAILABLE.withDescription("Unable to fallback, no fallback addresses found"); @VisibleForTesting static final RoundRobinEntry BUFFER_ENTRY = new RoundRobinEntry() { @@ -144,7 +147,6 @@ enum Mode { @Nullable private ManagedChannel lbCommChannel; - private boolean lbSentEmptyBackends = false; @Nullable private LbStream lbStream; @@ -289,7 +291,7 @@ private void maybeUseFallbackBackends() { } /** - * Populate the round-robin lists with the fallback backends. + * Populate backend servers to be used from the fallback backends. */ private void useFallbackBackends() { usingFallbackBackends = true; @@ -301,7 +303,7 @@ private void useFallbackBackends() { newDropList.add(null); newBackendAddrList.add(new BackendAddressGroup(eag, null)); } - useRoundRobinLists(newDropList, newBackendAddrList, null); + updateServerList(newDropList, newBackendAddrList, null); } private void shutdownLbComm() { @@ -418,9 +420,9 @@ GrpclbClientLoadRecorder getLoadRecorder() { } /** - * Populate the round-robin lists with the given values. + * Populate backend servers to be used based on the given list of addresses. */ - private void useRoundRobinLists( + private void updateServerList( List newDropList, List newBackendAddrList, @Nullable GrpclbClientLoadRecorder loadRecorder) { logger.log( @@ -470,7 +472,6 @@ private void useRoundRobinLists( final Subchannel subchannel; if (newBackendAddrList.isEmpty()) { if (subchannels.size() == 1) { - cancelFallbackTimer(); subchannel = subchannels.values().iterator().next(); subchannel.shutdown(); subchannels = Collections.emptyMap(); @@ -702,9 +703,8 @@ private void handleResponse(LoadBalanceResponse response) { } // Stop using fallback backends as soon as a new server list is received from the balancer. usingFallbackBackends = false; - lbSentEmptyBackends = serverList.getServersList().isEmpty(); cancelFallbackTimer(); - useRoundRobinLists(newDropList, newBackendAddrList, loadRecorder); + updateServerList(newDropList, newBackendAddrList, loadRecorder); maybeUpdatePicker(); } @@ -772,6 +772,24 @@ private void cleanUp() { private void maybeUpdatePicker() { List pickList; ConnectivityState state; + if (backendList.isEmpty()) { + if (balancerWorking) { + pickList = + Collections.singletonList( + new ErrorEntry(NO_AVAILABLE_BACKENDS_STATUS)); + state = TRANSIENT_FAILURE; + } else if (usingFallbackBackends) { + pickList = + Collections.singletonList( + new ErrorEntry(NO_FALLBACK_BACKENDS_FOUND_STATUS)); + state = TRANSIENT_FAILURE; + } else { // still waiting for balancer + pickList = Collections.singletonList(BUFFER_ENTRY); + state = CONNECTING; + } + maybeUpdatePicker(state, new RoundRobinPicker(dropList, pickList)); + return; + } switch (config.getMode()) { case ROUND_ROBIN: pickList = new ArrayList<>(backendList.size()); @@ -790,52 +808,40 @@ private void maybeUpdatePicker() { } } if (pickList.isEmpty()) { - if (error != null && !hasPending) { - pickList.add(new ErrorEntry(error)); - state = TRANSIENT_FAILURE; - } else { + if (hasPending) { pickList.add(BUFFER_ENTRY); state = CONNECTING; + } else { + pickList.add(new ErrorEntry(error)); + state = TRANSIENT_FAILURE; } } else { state = READY; } break; - case PICK_FIRST: - if (backendList.isEmpty()) { - if (lbSentEmptyBackends) { + case PICK_FIRST: { + checkState(backendList.size() == 1, "Excessive backend entries: %s", backendList); + BackendEntry onlyEntry = backendList.get(0); + ConnectivityStateInfo stateInfo = + onlyEntry.subchannel.getAttributes().get(STATE_INFO).get(); + state = stateInfo.getState(); + switch (state) { + case READY: + pickList = Collections.singletonList(onlyEntry); + break; + case TRANSIENT_FAILURE: pickList = - Collections.singletonList( - new ErrorEntry(NO_AVAILABLE_BACKENDS_STATUS)); - state = TRANSIENT_FAILURE; - } else { + Collections.singletonList(new ErrorEntry(stateInfo.getStatus())); + break; + case CONNECTING: pickList = Collections.singletonList(BUFFER_ENTRY); - // Have not received server addresses - state = CONNECTING; - } - } else { - checkState(backendList.size() == 1, "Excessive backend entries: %s", backendList); - BackendEntry onlyEntry = backendList.get(0); - ConnectivityStateInfo stateInfo = - onlyEntry.subchannel.getAttributes().get(STATE_INFO).get(); - state = stateInfo.getState(); - switch (state) { - case READY: - pickList = Collections.singletonList(onlyEntry); - break; - case TRANSIENT_FAILURE: - pickList = - Collections.singletonList(new ErrorEntry(stateInfo.getStatus())); - break; - case CONNECTING: - pickList = Collections.singletonList(BUFFER_ENTRY); - break; - default: - pickList = Collections.singletonList( - new IdleSubchannelEntry(onlyEntry.subchannel, syncContext)); - } + break; + default: + pickList = Collections.singletonList( + new IdleSubchannelEntry(onlyEntry.subchannel, syncContext)); } break; + } default: throw new AssertionError("Missing case for " + config.getMode()); } diff --git a/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java b/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java index 501735b30b1..edd422a8b76 100644 --- a/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java +++ b/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java @@ -1135,10 +1135,11 @@ public void grpclbWorking() { .returnSubchannel(same(subchannel2), eq(ConnectivityStateInfo.forNonError(READY))); verify(subchannelPool) .returnSubchannel(same(subchannel3), eq(ConnectivityStateInfo.forNonError(READY))); - inOrder.verify(helper).updateBalancingState(eq(CONNECTING), pickerCaptor.capture()); + inOrder.verify(helper).updateBalancingState(eq(TRANSIENT_FAILURE), pickerCaptor.capture()); RoundRobinPicker picker10 = (RoundRobinPicker) pickerCaptor.getValue(); assertThat(picker10.dropList).isEmpty(); - assertThat(picker10.pickList).containsExactly(BUFFER_ENTRY); + assertThat(picker10.pickList) + .containsExactly(new ErrorEntry(GrpclbState.NO_AVAILABLE_BACKENDS_STATUS)); assertFalse(oobChannel.isShutdown()); assertEquals(0, lbRequestObservers.size()); @@ -1251,8 +1252,6 @@ private void subtestGrpclbFallbackInitialTimeout(boolean timerExpires) { fakeClock.forwardTime(1, TimeUnit.MILLISECONDS); assertEquals(0, fakeClock.numPendingTasks(FALLBACK_MODE_TASK_FILTER)); - List fallbackList = - Arrays.asList(backendList.get(0), backendList.get(1)); assertThat(logs).containsExactly( "INFO: [grpclb-] Using fallback backends", "INFO: [grpclb-]" @@ -1264,7 +1263,7 @@ private void subtestGrpclbFallbackInitialTimeout(boolean timerExpires) { .inOrder(); // Fall back to the backends from resolver - fallbackTestVerifyUseOfFallbackBackendLists(inOrder, fallbackList); + fallbackTestVerifyUseOfFallbackBackendLists(inOrder, backendList); assertFalse(oobChannel.isShutdown()); verify(lbRequestObserver, never()).onCompleted(); @@ -1282,8 +1281,14 @@ private void subtestGrpclbFallbackInitialTimeout(boolean timerExpires) { if (timerExpires) { // Still in fallback logic, except that the backend list is empty - fallbackTestVerifyUseOfFallbackBackendLists( - inOrder, Collections.emptyList()); + for (Subchannel subchannel : mockSubchannels) { + verify(subchannelPool).returnSubchannel(eq(subchannel), any(ConnectivityStateInfo.class)); + } + inOrder.verify(helper).updateBalancingState(eq(TRANSIENT_FAILURE), pickerCaptor.capture()); + RoundRobinPicker picker = (RoundRobinPicker) pickerCaptor.getValue(); + assertThat(picker.dropList).isEmpty(); + assertThat(picker.pickList) + .containsExactly(new ErrorEntry(GrpclbState.NO_FALLBACK_BACKENDS_FOUND_STATUS)); } ////////////////////////////////////////////////////////////////