Skip to content

Commit 2e260a4

Browse files
committed
util: Graceful switch to new LB when leaving CONNECTING
Previously it would wait for the new LB to enter READY. However, that prevents there being an upper-bound on how long the old policy will continue to be used. The point of graceful switch is to avoid RPCs seeing increased latency when we swap config. We don't want it to prevent the system from becoming eventually consistent.
1 parent 7507a9e commit 2e260a4

File tree

2 files changed

+24
-7
lines changed

2 files changed

+24
-7
lines changed

util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@
3838
/**
3939
* A load balancer that gracefully swaps to a new lb policy. If the channel is currently in a state
4040
* other than READY, the new policy will be swapped into place immediately. Otherwise, the channel
41-
* will keep using the old policy until the new policy reports READY or the old policy exits READY.
41+
* will keep using the old policy until the new policy leaves CONNECTING or the old policy exits
42+
* READY.
4243
*
4344
* <p>The child balancer and configuration is specified using service config. Config objects are
4445
* generally created by calling {@link #parseLoadBalancingPolicyConfig(List)} from a
@@ -147,7 +148,7 @@ public void updateBalancingState(ConnectivityState newState, SubchannelPicker ne
147148
checkState(currentLbIsReady, "there's pending lb while current lb has been out of READY");
148149
pendingState = newState;
149150
pendingPicker = newPicker;
150-
if (newState == ConnectivityState.READY) {
151+
if (newState != ConnectivityState.CONNECTING) {
151152
swap();
152153
}
153154
} else if (lb == currentLb) {

util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java

+21-5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import static com.google.common.truth.Truth.assertThat;
2020
import static io.grpc.ConnectivityState.CONNECTING;
21+
import static io.grpc.ConnectivityState.IDLE;
2122
import static io.grpc.ConnectivityState.READY;
2223
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
2324
import static io.grpc.util.GracefulSwitchLoadBalancer.BUFFER_PICKER;
@@ -32,6 +33,7 @@
3233
import static org.mockito.Mockito.when;
3334

3435
import com.google.common.testing.EqualsTester;
36+
import io.grpc.ConnectivityState;
3537
import io.grpc.ConnectivityStateInfo;
3638
import io.grpc.EquivalentAddressGroup;
3739
import io.grpc.LoadBalancer;
@@ -363,7 +365,21 @@ public void createSubchannelForwarded() {
363365
}
364366

365367
@Test
366-
public void updateBalancingStateIsGraceful() {
368+
public void updateBalancingStateIsGraceful_Ready() {
369+
updateBalancingStateIsGraceful(READY);
370+
}
371+
372+
@Test
373+
public void updateBalancingStateIsGraceful_TransientFailure() {
374+
updateBalancingStateIsGraceful(TRANSIENT_FAILURE);
375+
}
376+
377+
@Test
378+
public void updateBalancingStateIsGraceful_Idle() {
379+
updateBalancingStateIsGraceful(IDLE);
380+
}
381+
382+
public void updateBalancingStateIsGraceful(ConnectivityState swapsOnState) {
367383
assertIsOk(gracefulSwitchLb.acceptResolvedAddresses(addressesBuilder()
368384
.setLoadBalancingPolicyConfig(createConfig(lbPolicies[0], new Object()))
369385
.build()));
@@ -392,11 +408,11 @@ public void updateBalancingStateIsGraceful() {
392408
helper2.updateBalancingState(CONNECTING, picker);
393409
verify(mockHelper, never()).updateBalancingState(CONNECTING, picker);
394410

395-
// lb2 reports READY
411+
// lb2 reports swapsOnState
396412
SubchannelPicker picker2 = mock(SubchannelPicker.class);
397-
helper2.updateBalancingState(READY, picker2);
413+
helper2.updateBalancingState(swapsOnState, picker2);
398414
verify(lb0).shutdown();
399-
verify(mockHelper).updateBalancingState(READY, picker2);
415+
verify(mockHelper).updateBalancingState(swapsOnState, picker2);
400416

401417
assertIsOk(gracefulSwitchLb.acceptResolvedAddresses(addressesBuilder()
402418
.setLoadBalancingPolicyConfig(createConfig(lbPolicies[3], new Object()))
@@ -407,7 +423,7 @@ public void updateBalancingStateIsGraceful() {
407423
helper3.updateBalancingState(CONNECTING, picker3);
408424
verify(mockHelper, never()).updateBalancingState(CONNECTING, picker3);
409425

410-
// lb2 out of READY
426+
// lb2 out of swapsOnState
411427
picker2 = mock(SubchannelPicker.class);
412428
helper2.updateBalancingState(CONNECTING, picker2);
413429
verify(mockHelper, never()).updateBalancingState(CONNECTING, picker2);

0 commit comments

Comments
 (0)