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

Addresses a problem where we initially only resolve addresses to the backends, but not the load balancer and then later resolve addresses to both. In this situation the fallback timer was started during the second instance even if it resulted in the timer later failing as we were already using fallback backends.

This change assures that a fallback time is only ever started if we are not already using the fallback backends.

This is a follow-up fix to #8253.
This commit is contained in:
Terry Wilson 2021-11-02 12:47:47 -07:00 committed by GitHub
parent 746501dff6
commit c1e19af86d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 2 deletions

View File

@ -287,8 +287,9 @@ final class GrpclbState {
cancelLbRpcRetryTimer(); cancelLbRpcRetryTimer();
startLbRpc(); startLbRpc();
} }
// Start the fallback timer if it's never started // Start the fallback timer if it's never started and we are not already using fallback
if (fallbackTimer == null) { // backends.
if (fallbackTimer == null && !usingFallbackBackends) {
fallbackTimer = syncContext.schedule( fallbackTimer = syncContext.schedule(
new FallbackModeTask(BALANCER_TIMEOUT_STATUS), FALLBACK_TIMEOUT_MS, new FallbackModeTask(BALANCER_TIMEOUT_STATUS), FALLBACK_TIMEOUT_MS,
TimeUnit.MILLISECONDS, timerService); TimeUnit.MILLISECONDS, timerService);

View File

@ -1462,6 +1462,33 @@ public class GrpclbLoadBalancerTest {
.updateBalancingState(eq(TRANSIENT_FAILURE), any(SubchannelPicker.class)); .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 @Test
public void grpclbFallback_balancerLost() { public void grpclbFallback_balancerLost() {
subtestGrpclbFallbackConnectionLost(true, false); subtestGrpclbFallbackConnectionLost(true, false);