diff --git a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java index e36eec1ff2..1dc4fb6750 100644 --- a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java @@ -19,7 +19,6 @@ package io.grpc.util; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import io.grpc.ConnectivityState; @@ -66,19 +65,6 @@ public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { public void shutdown() {} }; - @VisibleForTesting - static final SubchannelPicker BUFFER_PICKER = new SubchannelPicker() { - @Override - public PickResult pickSubchannel(PickSubchannelArgs args) { - return PickResult.withNoResult(); - } - - @Override - public String toString() { - return "BUFFER_PICKER"; - } - }; - private final Helper helper; // While the new policy is not fully switched on, the pendingLb is handling new updates from name @@ -128,7 +114,7 @@ public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { pendingLb = defaultBalancer; pendingBalancerFactory = null; pendingState = ConnectivityState.CONNECTING; - pendingPicker = BUFFER_PICKER; + pendingPicker = new FixedResultPicker(PickResult.withNoResult()); if (newBalancerFactory.equals(currentBalancerFactory)) { return; diff --git a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java index 5192d6a2a6..843e16194c 100644 --- a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java @@ -21,7 +21,6 @@ import static io.grpc.ConnectivityState.CONNECTING; import static io.grpc.ConnectivityState.IDLE; import static io.grpc.ConnectivityState.READY; import static io.grpc.ConnectivityState.TRANSIENT_FAILURE; -import static io.grpc.util.GracefulSwitchLoadBalancer.BUFFER_PICKER; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.inOrder; @@ -521,7 +520,11 @@ public class GracefulSwitchLoadBalancerTest { helper0.updateBalancingState(CONNECTING, picker); verify(mockHelper, never()).updateBalancingState(CONNECTING, picker); - inOrder.verify(mockHelper).updateBalancingState(CONNECTING, BUFFER_PICKER); + ArgumentCaptor pickerCaptor = ArgumentCaptor.forClass(SubchannelPicker.class); + inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), pickerCaptor.capture()); + assertThat(pickerCaptor.getValue().pickSubchannel(mock(PickSubchannelArgs.class)).hasResult()) + .isFalse(); + inOrder.verify(lb0).shutdown(); // shutdown after update picker = mock(SubchannelPicker.class); diff --git a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java index 9823501dcd..08d4863d19 100644 --- a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java @@ -522,8 +522,7 @@ public class PriorityLoadBalancerTest { .setLoadBalancingPolicyConfig(priorityLbConfig) .build()); // Nothing important about this verify, other than to provide a baseline - verify(helper, times(2)) - .updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult())); + verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult())); assertThat(fooBalancers).hasSize(1); assertThat(fooHelpers).hasSize(1); Helper helper0 = Iterables.getOnlyElement(fooHelpers); @@ -539,7 +538,7 @@ public class PriorityLoadBalancerTest { helper0.updateBalancingState( CONNECTING, EMPTY_PICKER); - verify(helper, times(3)) + verify(helper, times(2)) .updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult())); // failover happens @@ -805,7 +804,7 @@ public class PriorityLoadBalancerTest { .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) .build()); - verify(helper, times(2)).updateBalancingState(eq(CONNECTING), isA(SubchannelPicker.class)); + verify(helper).updateBalancingState(eq(CONNECTING), isA(SubchannelPicker.class)); // LB shutdown and subchannel state change can happen simultaneously. If shutdown runs first, // any further balancing state update should be ignored. @@ -843,7 +842,7 @@ public class PriorityLoadBalancerTest { .setLoadBalancingPolicyConfig(priorityLbConfig) .build()); - verify(helper, times(6)).updateBalancingState(any(), any()); + verify(helper, times(4)).updateBalancingState(any(), any()); } private void assertLatestConnectivityState(ConnectivityState expectedState) {