diff --git a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java index a63a641b03..72c41886ad 100644 --- a/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java @@ -44,28 +44,14 @@ import javax.annotation.concurrent.NotThreadSafe; * generally created by calling {@link #parseLoadBalancingPolicyConfig(List)} from a * {@link io.grpc.LoadBalancerProvider#parseLoadBalancingPolicyConfig * provider's parseLoadBalancingPolicyConfig()} implementation. - * - *

Alternatively, the balancer may {@link #switchTo(LoadBalancer.Factory) switch to} a policy - * prior to {@link - * LoadBalancer#handleResolvedAddresses(ResolvedAddresses) handling resolved addresses} for the - * first time. This causes graceful switch to ignore the service config and pass through the - * resolved addresses directly to the child policy. */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/5999") @NotThreadSafe // Must be accessed in SynchronizationContext public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { private final LoadBalancer defaultBalancer = new LoadBalancer() { @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { - // Most LB policies using this class will receive child policy configuration within the - // service config, so they are naturally calling switchTo() just before - // handleResolvedAddresses(), within their own handleResolvedAddresses(). If switchTo() is - // not called immediately after construction that does open up potential for bugs in the - // parent policies, where they fail to call switchTo(). So we will use the exception to try - // to notice those bugs quickly, as it will fail very loudly. - throw new IllegalStateException( - "GracefulSwitchLoadBalancer must switch to a load balancing policy before handling" - + " ResolvedAddresses"); + public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + throw new AssertionError("real LB is called instead"); } @Override @@ -104,7 +90,6 @@ public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { private LoadBalancer pendingLb = defaultBalancer; private ConnectivityState pendingState; private SubchannelPicker pendingPicker; - private boolean switchToCalled; private boolean currentLbIsReady; @@ -114,10 +99,6 @@ public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { @Override public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { - if (switchToCalled) { - delegate().handleResolvedAddresses(resolvedAddresses); - return; - } Config config = (Config) resolvedAddresses.getLoadBalancingPolicyConfig(); switchToInternal(config.childFactory); delegate().handleResolvedAddresses( @@ -128,9 +109,6 @@ public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { @Override public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { - if (switchToCalled) { - return delegate().acceptResolvedAddresses(resolvedAddresses); - } Config config = (Config) resolvedAddresses.getLoadBalancingPolicyConfig(); switchToInternal(config.childFactory); return delegate().acceptResolvedAddresses( @@ -139,19 +117,6 @@ public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { .build()); } - /** - * Gracefully switch to a new policy defined by the given factory, if the given factory isn't - * equal to the current one. - * - * @deprecated Use {@code parseLoadBalancingPolicyConfig()} and pass the configuration to - * {@link io.grpc.LoadBalancer.ResolvedAddresses.Builder#setLoadBalancingPolicyConfig} - */ - @Deprecated - public void switchTo(LoadBalancer.Factory newBalancerFactory) { - switchToCalled = true; - switchToInternal(newBalancerFactory); - } - private void switchToInternal(LoadBalancer.Factory newBalancerFactory) { checkNotNull(newBalancerFactory, "newBalancerFactory"); diff --git a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java index f31443ace7..9a4f569c14 100644 --- a/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java @@ -83,453 +83,6 @@ public class GracefulSwitchLoadBalancerTest { new FakeLoadBalancerProvider("lb_policy_3"), }; - // OLD TESTS - - @Test - @Deprecated - public void switchTo_canHandleEmptyAddressListFromNameResolutionForwardedToLatestPolicy() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isFalse(); - when(lb0.canHandleEmptyAddressListFromNameResolution()).thenReturn(true); - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isTrue(); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isFalse(); - - when(lb1.canHandleEmptyAddressListFromNameResolution()).thenReturn(true); - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isTrue(); - - gracefulSwitchLb.switchTo(lbPolicies[2]); - LoadBalancer lb2 = balancers.get(lbPolicies[2]); - - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isFalse(); - - when(lb2.canHandleEmptyAddressListFromNameResolution()).thenReturn(true); - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isTrue(); - } - - @Test - @Deprecated - public void switchTo_handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - - ResolvedAddresses addresses = newFakeAddresses(); - gracefulSwitchLb.handleResolvedAddresses(addresses); - verify(lb0).handleResolvedAddresses(addresses); - gracefulSwitchLb.handleNameResolutionError(Status.DATA_LOSS); - verify(lb0).handleNameResolutionError(Status.DATA_LOSS); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - addresses = newFakeAddresses(); - gracefulSwitchLb.handleResolvedAddresses(addresses); - verify(lb0, never()).handleResolvedAddresses(addresses); - verify(lb1).handleResolvedAddresses(addresses); - gracefulSwitchLb.handleNameResolutionError(Status.ALREADY_EXISTS); - verify(lb0, never()).handleNameResolutionError(Status.ALREADY_EXISTS); - verify(lb1).handleNameResolutionError(Status.ALREADY_EXISTS); - - gracefulSwitchLb.switchTo(lbPolicies[2]); - verify(lb1).shutdown(); - LoadBalancer lb2 = balancers.get(lbPolicies[2]); - addresses = newFakeAddresses(); - gracefulSwitchLb.handleResolvedAddresses(addresses); - verify(lb0, never()).handleResolvedAddresses(addresses); - verify(lb1, never()).handleResolvedAddresses(addresses); - verify(lb2).handleResolvedAddresses(addresses); - gracefulSwitchLb.handleNameResolutionError(Status.CANCELLED); - verify(lb0, never()).handleNameResolutionError(Status.CANCELLED); - verify(lb1, never()).handleNameResolutionError(Status.CANCELLED); - verify(lb2).handleNameResolutionError(Status.CANCELLED); - - verifyNoMoreInteractions(lb0, lb1, lb2); - } - - @Test - @Deprecated - public void switchTo_acceptResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - - ResolvedAddresses addresses = newFakeAddresses(); - gracefulSwitchLb.acceptResolvedAddresses(addresses); - verify(lb0).acceptResolvedAddresses(addresses); - gracefulSwitchLb.handleNameResolutionError(Status.DATA_LOSS); - verify(lb0).handleNameResolutionError(Status.DATA_LOSS); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - addresses = newFakeAddresses(); - gracefulSwitchLb.acceptResolvedAddresses(addresses); - verify(lb0, never()).acceptResolvedAddresses(addresses); - verify(lb1).acceptResolvedAddresses(addresses); - gracefulSwitchLb.handleNameResolutionError(Status.ALREADY_EXISTS); - verify(lb0, never()).handleNameResolutionError(Status.ALREADY_EXISTS); - verify(lb1).handleNameResolutionError(Status.ALREADY_EXISTS); - - gracefulSwitchLb.switchTo(lbPolicies[2]); - verify(lb1).shutdown(); - LoadBalancer lb2 = balancers.get(lbPolicies[2]); - addresses = newFakeAddresses(); - gracefulSwitchLb.acceptResolvedAddresses(addresses); - verify(lb0, never()).acceptResolvedAddresses(addresses); - verify(lb1, never()).acceptResolvedAddresses(addresses); - verify(lb2).acceptResolvedAddresses(addresses); - gracefulSwitchLb.handleNameResolutionError(Status.CANCELLED); - verify(lb0, never()).handleNameResolutionError(Status.CANCELLED); - verify(lb1, never()).handleNameResolutionError(Status.CANCELLED); - verify(lb2).handleNameResolutionError(Status.CANCELLED); - - verifyNoMoreInteractions(lb0, lb1, lb2); - } - - @Test - @Deprecated - public void switchTo_shutdownTriggeredWhenSwitchAndForwardedWhenSwitchLbShutdown() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - verify(lb1, never()).shutdown(); - - gracefulSwitchLb.switchTo(lbPolicies[2]); - verify(lb1).shutdown(); - LoadBalancer lb2 = balancers.get(lbPolicies[2]); - verify(lb0, never()).shutdown(); - helpers.get(lb2).updateBalancingState(READY, mock(SubchannelPicker.class)); - verify(lb0).shutdown(); - - gracefulSwitchLb.switchTo(lbPolicies[3]); - LoadBalancer lb3 = balancers.get(lbPolicies[3]); - verify(lb2, never()).shutdown(); - verify(lb3, never()).shutdown(); - - gracefulSwitchLb.shutdown(); - verify(lb2).shutdown(); - verify(lb3).shutdown(); - - verifyNoMoreInteractions(lb0, lb1, lb2, lb3); - } - - @Test - @Deprecated - public void switchTo_requestConnectionForwardedToLatestPolicies() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - - gracefulSwitchLb.requestConnection(); - verify(lb0).requestConnection(); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - gracefulSwitchLb.requestConnection(); - verify(lb1).requestConnection(); - - gracefulSwitchLb.switchTo(lbPolicies[2]); - verify(lb1).shutdown(); - LoadBalancer lb2 = balancers.get(lbPolicies[2]); - gracefulSwitchLb.requestConnection(); - verify(lb2).requestConnection(); - - // lb2 reports READY - helpers.get(lb2).updateBalancingState(READY, mock(SubchannelPicker.class)); - verify(lb0).shutdown(); - - gracefulSwitchLb.requestConnection(); - verify(lb2, times(2)).requestConnection(); - - gracefulSwitchLb.switchTo(lbPolicies[3]); - LoadBalancer lb3 = balancers.get(lbPolicies[3]); - gracefulSwitchLb.requestConnection(); - verify(lb3).requestConnection(); - - verifyNoMoreInteractions(lb0, lb1, lb2, lb3); - } - - @Test - @Deprecated - public void switchTo_createSubchannelForwarded() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - - CreateSubchannelArgs createSubchannelArgs = newFakeCreateSubchannelArgs(); - helper0.createSubchannel(createSubchannelArgs); - verify(mockHelper).createSubchannel(createSubchannelArgs); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - Helper helper1 = helpers.get(lb1); - createSubchannelArgs = newFakeCreateSubchannelArgs(); - helper1.createSubchannel(createSubchannelArgs); - verify(mockHelper).createSubchannel(createSubchannelArgs); - - createSubchannelArgs = newFakeCreateSubchannelArgs(); - helper0.createSubchannel(createSubchannelArgs); - verify(mockHelper).createSubchannel(createSubchannelArgs); - - verifyNoMoreInteractions(lb0, lb1); - } - - @Test - @Deprecated - public void switchTo_updateBalancingStateIsGraceful() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - verify(mockHelper).updateBalancingState(READY, picker); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - Helper helper1 = helpers.get(lb1); - picker = mock(SubchannelPicker.class); - helper1.updateBalancingState(CONNECTING, picker); - verify(mockHelper, never()).updateBalancingState(CONNECTING, picker); - - gracefulSwitchLb.switchTo(lbPolicies[2]); - verify(lb1).shutdown(); - LoadBalancer lb2 = balancers.get(lbPolicies[2]); - Helper helper2 = helpers.get(lb2); - picker = mock(SubchannelPicker.class); - helper2.updateBalancingState(CONNECTING, picker); - verify(mockHelper, never()).updateBalancingState(CONNECTING, picker); - - // lb2 reports READY - SubchannelPicker picker2 = mock(SubchannelPicker.class); - helper2.updateBalancingState(READY, picker2); - verify(lb0).shutdown(); - verify(mockHelper).updateBalancingState(READY, picker2); - - gracefulSwitchLb.switchTo(lbPolicies[3]); - LoadBalancer lb3 = balancers.get(lbPolicies[3]); - Helper helper3 = helpers.get(lb3); - SubchannelPicker picker3 = mock(SubchannelPicker.class); - helper3.updateBalancingState(CONNECTING, picker3); - verify(mockHelper, never()).updateBalancingState(CONNECTING, picker3); - - // lb2 out of READY - picker2 = mock(SubchannelPicker.class); - helper2.updateBalancingState(CONNECTING, picker2); - verify(mockHelper, never()).updateBalancingState(CONNECTING, picker2); - verify(mockHelper).updateBalancingState(CONNECTING, picker3); - verify(lb2).shutdown(); - - picker3 = mock(SubchannelPicker.class); - helper3.updateBalancingState(CONNECTING, picker3); - verify(mockHelper).updateBalancingState(CONNECTING, picker3); - - verifyNoMoreInteractions(lb0, lb1, lb2, lb3); - } - - @Test - @Deprecated - public void switchTo_switchWhileOldPolicyIsNotReady() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(CONNECTING, picker); - - verify(lb0, never()).shutdown(); - gracefulSwitchLb.switchTo(lbPolicies[1]); - verify(lb0).shutdown(); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - - Helper helper1 = helpers.get(lb1); - picker = mock(SubchannelPicker.class); - helper1.updateBalancingState(CONNECTING, picker); - verify(mockHelper).updateBalancingState(CONNECTING, picker); - - verify(lb1, never()).shutdown(); - gracefulSwitchLb.switchTo(lbPolicies[2]); - verify(lb1).shutdown(); - LoadBalancer lb2 = balancers.get(lbPolicies[2]); - - verifyNoMoreInteractions(lb0, lb1, lb2); - } - - @Test - @Deprecated - public void switchTo_switchWhileOldPolicyGoesFromReadyToNotReady() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - verify(lb0, never()).shutdown(); - - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - Helper helper1 = helpers.get(lb1); - SubchannelPicker picker1 = mock(SubchannelPicker.class); - helper1.updateBalancingState(CONNECTING, picker1); - verify(mockHelper, never()).updateBalancingState(CONNECTING, picker1); - - picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(CONNECTING, picker); - verify(lb0).shutdown(); - verify(mockHelper, never()).updateBalancingState(CONNECTING, picker); - verify(mockHelper).updateBalancingState(CONNECTING, picker1); - - picker1 = mock(SubchannelPicker.class); - helper1.updateBalancingState(READY, picker1); - verify(mockHelper).updateBalancingState(READY, picker1); - - verifyNoMoreInteractions(lb0, lb1); - } - - @Test - @Deprecated - public void switchTo_switchWhileOldPolicyGoesFromReadyToNotReadyWhileNewPolicyStillIdle() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - InOrder inOrder = inOrder(lb0, mockHelper); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - verify(lb0, never()).shutdown(); - - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - Helper helper1 = helpers.get(lb1); - - picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(CONNECTING, picker); - - verify(mockHelper, never()).updateBalancingState(CONNECTING, picker); - inOrder.verify(mockHelper).updateBalancingState(CONNECTING, BUFFER_PICKER); - inOrder.verify(lb0).shutdown(); // shutdown after update - - picker = mock(SubchannelPicker.class); - helper1.updateBalancingState(CONNECTING, picker); - inOrder.verify(mockHelper).updateBalancingState(CONNECTING, picker); - - inOrder.verifyNoMoreInteractions(); - verifyNoMoreInteractions(lb1); - } - - @Test - @Deprecated - public void switchTo_newPolicyNameTheSameAsPendingPolicy_shouldHaveNoEffect() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - assertThat(balancers.get(lbPolicies[1])).isSameInstanceAs(lb1); - - verifyNoMoreInteractions(lb0, lb1); - } - - @Test - @Deprecated - public void switchTo_newPolicyNameTheSameAsCurrentPolicy_shouldShutdownPendingLb() { - gracefulSwitchLb.switchTo(lbPolicies[0]); - LoadBalancer lb0 = balancers.get(lbPolicies[0]); - - gracefulSwitchLb.switchTo(lbPolicies[0]); - assertThat(balancers.get(lbPolicies[0])).isSameInstanceAs(lb0); - - Helper helper0 = helpers.get(lb0); - SubchannelPicker picker = mock(SubchannelPicker.class); - helper0.updateBalancingState(READY, picker); - - gracefulSwitchLb.switchTo(lbPolicies[1]); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - - gracefulSwitchLb.switchTo(lbPolicies[0]); - verify(lb1).shutdown(); - assertThat(balancers.get(lbPolicies[0])).isSameInstanceAs(lb0); - - verifyNoMoreInteractions(lb0, lb1); - } - - - @Test - @Deprecated - public void switchTo_newLbFactoryEqualToOldOneShouldHaveNoEffect() { - final List balancers = new ArrayList<>(); - - final class LoadBalancerFactoryWithId extends LoadBalancer.Factory { - final int id; - - LoadBalancerFactoryWithId(int id) { - this.id = id; - } - - @Override - public LoadBalancer newLoadBalancer(Helper helper) { - LoadBalancer balancer = mock(LoadBalancer.class); - balancers.add(balancer); - return balancer; - } - - @Override - public boolean equals(Object o) { - if (!(o instanceof LoadBalancerFactoryWithId)) { - return false; - } - LoadBalancerFactoryWithId that = (LoadBalancerFactoryWithId) o; - return id == that.id; - } - - @Override - public int hashCode() { - return id; - } - } - - gracefulSwitchLb.switchTo(new LoadBalancerFactoryWithId(0)); - assertThat(balancers).hasSize(1); - LoadBalancer lb0 = balancers.get(0); - - gracefulSwitchLb.switchTo(new LoadBalancerFactoryWithId(0)); - assertThat(balancers).hasSize(1); - - gracefulSwitchLb.switchTo(new LoadBalancerFactoryWithId(1)); - assertThat(balancers).hasSize(2); - LoadBalancer lb1 = balancers.get(1); - verify(lb0).shutdown(); - - verifyNoMoreInteractions(lb0, lb1); - } - - // END OF OLD TESTS - @Test public void transientFailureOnInitialResolutionError() { gracefulSwitchLb.handleNameResolutionError(Status.DATA_LOSS);