Skip to content
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.

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

grpclb: fallback timer only when not already using fallback backends. #8646

Merged
merged 4 commits into from Nov 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions grpclb/src/main/java/io/grpc/grpclb/GrpclbState.java
Expand Up @@ -287,8 +287,9 @@ void handleAddresses(
cancelLbRpcRetryTimer();
startLbRpc();
}
// Start the fallback timer if it's never started
if (fallbackTimer == null) {
// Start the fallback timer if it's never started and we are not already using fallback
// backends.
if (fallbackTimer == null && !usingFallbackBackends) {
fallbackTimer = syncContext.schedule(
new FallbackModeTask(BALANCER_TIMEOUT_STATUS), FALLBACK_TIMEOUT_MS,
TimeUnit.MILLISECONDS, timerService);
Expand Down
27 changes: 27 additions & 0 deletions grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java
Expand Up @@ -1462,6 +1462,33 @@ public void grpclbFallback_noBalancerAddress() {
.updateBalancingState(eq(TRANSIENT_FAILURE), any(SubchannelPicker.class));
}

/**
* A test for a situation where we first only get backend addresses resolved and then in a
* later name resolution get both backend and load balancer addresses. The first instance
* will switch us to using fallback backends and it is important that in the second instance
* we do not start a fallback timer as it will fail when it triggers if the fallback backends
* are already in use.
*/
@Test
public void grpclbFallback_noTimerWhenAlreadyInFallback() {
// Initially we only get backend addresses without any LB ones. This should get us to use
// fallback backends from the start as we won't be able to even talk to the load balancer.
// No fallback timer would be started as we already started to use fallback backends.
deliverResolvedAddresses(createResolvedBalancerAddresses(1),
Collections.<EquivalentAddressGroup>emptyList());
assertEquals(0, fakeClock.numPendingTasks(FALLBACK_MODE_TASK_FILTER));

// Later a new name resolution call happens and we get both backend and LB addresses. Since we
// are already operating with fallback backends a fallback timer should not be started to move
// us to fallback mode.
deliverResolvedAddresses(Collections.<EquivalentAddressGroup>emptyList(),
createResolvedBalancerAddresses(1));

// If a fallback timer is started it will eventually throw an exception when it tries to switch
// us to using fallback backends when we already are using them.
assertEquals(0, fakeClock.numPendingTasks(FALLBACK_MODE_TASK_FILTER));
}

@Test
public void grpclbFallback_balancerLost() {
subtestGrpclbFallbackConnectionLost(true, false);
Expand Down