Skip to content

grpclb: turn into TRANSIENT_FAILURE if given (by balancer or fallback) an empty list of addresses #7960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 13, 2021
Merged

grpclb: turn into TRANSIENT_FAILURE if given (by balancer or fallback) an empty list of addresses #7960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bbc7e89
Turn LB state into TRANSIENT_FAILURE if an empty list of backend addr…
voidzcy Mar 12, 2021
da4bbb2
Merge branch 'master' of github.com:grpc/grpc-java into bugfix/grpclb…
voidzcy Mar 12, 2021
677dc92
Impove style.
voidzcy Mar 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 49 additions & 43 deletions grpclb/src/main/java/io/grpc/grpclb/GrpclbState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -144,7 +147,6 @@ enum Mode {

@Nullable
private ManagedChannel lbCommChannel;
private boolean lbSentEmptyBackends = false;

@Nullable
private LbStream lbStream;
Expand Down Expand Up @@ -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;
Expand All @@ -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() {
Expand Down Expand Up @@ -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<DropEntry> newDropList, List<BackendAddressGroup> newBackendAddrList,
@Nullable GrpclbClientLoadRecorder loadRecorder) {
logger.log(
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -772,6 +772,24 @@ private void cleanUp() {
private void maybeUpdatePicker() {
List<RoundRobinEntry> pickList;
ConnectivityState state;
if (backendList.isEmpty()) {
if (balancerWorking) {
pickList =
Collections.<RoundRobinEntry>singletonList(
new ErrorEntry(NO_AVAILABLE_BACKENDS_STATUS));
state = TRANSIENT_FAILURE;
} else if (usingFallbackBackends) {
pickList =
Collections.<RoundRobinEntry>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());
Expand All @@ -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.<RoundRobinEntry>singletonList(onlyEntry);
break;
case TRANSIENT_FAILURE:
pickList =
Collections.<RoundRobinEntry>singletonList(
new ErrorEntry(NO_AVAILABLE_BACKENDS_STATUS));
state = TRANSIENT_FAILURE;
} else {
Collections.<RoundRobinEntry>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.<RoundRobinEntry>singletonList(onlyEntry);
break;
case TRANSIENT_FAILURE:
pickList =
Collections.<RoundRobinEntry>singletonList(new ErrorEntry(stateInfo.getStatus()));
break;
case CONNECTING:
pickList = Collections.singletonList(BUFFER_ENTRY);
break;
default:
pickList = Collections.<RoundRobinEntry>singletonList(
new IdleSubchannelEntry(onlyEntry.subchannel, syncContext));
}
break;
default:
pickList = Collections.<RoundRobinEntry>singletonList(
new IdleSubchannelEntry(onlyEntry.subchannel, syncContext));
}
break;
}
default:
throw new AssertionError("Missing case for " + config.getMode());
}
Expand Down
19 changes: 12 additions & 7 deletions grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -1251,8 +1252,6 @@ private void subtestGrpclbFallbackInitialTimeout(boolean timerExpires) {
fakeClock.forwardTime(1, TimeUnit.MILLISECONDS);

assertEquals(0, fakeClock.numPendingTasks(FALLBACK_MODE_TASK_FILTER));
List<EquivalentAddressGroup> fallbackList =
Arrays.asList(backendList.get(0), backendList.get(1));
assertThat(logs).containsExactly(
"INFO: [grpclb-<api.google.com>] Using fallback backends",
"INFO: [grpclb-<api.google.com>]"
Expand All @@ -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();
Expand All @@ -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.<EquivalentAddressGroup>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));
}

////////////////////////////////////////////////////////////////
Expand Down