From 70a29fbfe3b8469ff323f036e031db48b7a32b84 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Tue, 26 Jul 2022 09:23:37 -0700 Subject: [PATCH] Add LoadBalancer.acceptResolvedAddresses() (#8742) Introduces a new acceptResolvedAddresses() to the LoadBalancer. This will now be the preferred way to handle addresses from the NameResolver. The existing handleResolvedAddresses() will eventually be deprecated. The new method returns a boolean based on the LoadBalancers ability to use the provided addresses. If it does not accept them, false is returned. LoadBalancer implementations using the new method should no longer implement the canHandleEmptyAddressListFromNameResolution(), which will eventually be removed, along with handleResolvedAddresses(). Backward compatibility will be maintained so existing load balancers using handleResolvedAddresses() will continue to work. Additionally the previously deprecated handleResolvedAddressGroups() method is removed. --- api/src/main/java/io/grpc/LoadBalancer.java | 49 ++++---- .../test/java/io/grpc/LoadBalancerTest.java | 54 +++++---- .../AutoConfiguredLoadBalancerFactory.java | 42 +++---- .../io/grpc/internal/ManagedChannelImpl.java | 9 +- .../grpc/internal/PickFirstLoadBalancer.java | 4 +- .../io/grpc/util/ForwardingLoadBalancer.java | 14 +-- .../grpc/util/GracefulSwitchLoadBalancer.java | 4 +- .../io/grpc/util/RoundRobinLoadBalancer.java | 4 +- ...AutoConfiguredLoadBalancerFactoryTest.java | 108 +++++++++--------- .../ManagedChannelImplIdlenessTest.java | 3 +- .../grpc/internal/ManagedChannelImplTest.java | 46 +++----- .../internal/PickFirstLoadBalancerTest.java | 22 ++-- .../ServiceConfigErrorHandlingTest.java | 51 +++++---- .../util/GracefulSwitchLoadBalancerTest.java | 48 ++------ .../grpc/util/RoundRobinLoadBalancerTest.java | 20 ++-- .../io/grpc/grpclb/GrpclbLoadBalancer.java | 11 +- .../grpc/grpclb/GrpclbLoadBalancerTest.java | 2 +- .../RpcBehaviorLoadBalancerProvider.java | 6 +- .../RpcBehaviorLoadBalancerProviderTest.java | 4 +- .../io/grpc/rls/LbPolicyConfiguration.java | 2 +- .../java/io/grpc/rls/RlsLoadBalancer.java | 4 +- .../io/grpc/rls/CachingRlsLbClientTest.java | 4 +- .../java/io/grpc/rls/RlsLoadBalancerTest.java | 2 +- .../HealthCheckingLoadBalancerFactory.java | 4 +- ...HealthCheckingLoadBalancerFactoryTest.java | 97 ++++++++-------- .../java/io/grpc/xds/CdsLoadBalancer2.java | 7 +- .../io/grpc/xds/ClusterImplLoadBalancer.java | 9 +- .../grpc/xds/ClusterManagerLoadBalancer.java | 15 +-- .../grpc/xds/ClusterResolverLoadBalancer.java | 12 +- .../io/grpc/xds/LeastRequestLoadBalancer.java | 4 +- .../io/grpc/xds/PriorityLoadBalancer.java | 6 +- .../io/grpc/xds/RingHashLoadBalancer.java | 6 +- .../grpc/xds/WeightedTargetLoadBalancer.java | 15 +-- .../io/grpc/xds/WrrLocalityLoadBalancer.java | 6 +- .../io/grpc/xds/CdsLoadBalancer2Test.java | 5 +- .../grpc/xds/ClusterImplLoadBalancerTest.java | 7 +- .../xds/ClusterManagerLoadBalancerTest.java | 6 +- .../xds/ClusterResolverLoadBalancerTest.java | 5 +- .../xds/LeastRequestLoadBalancerTest.java | 26 ++--- .../xds/MetadataLoadBalancerProvider.java | 6 +- .../io/grpc/xds/PriorityLoadBalancerTest.java | 32 +++--- .../io/grpc/xds/RingHashLoadBalancerTest.java | 50 ++++---- .../xds/WeightedTargetLoadBalancerTest.java | 19 +-- .../grpc/xds/WrrLocalityLoadBalancerTest.java | 6 +- 44 files changed, 414 insertions(+), 442 deletions(-) diff --git a/api/src/main/java/io/grpc/LoadBalancer.java b/api/src/main/java/io/grpc/LoadBalancer.java index 8d74216b36..ebe30e7d6d 100644 --- a/api/src/main/java/io/grpc/LoadBalancer.java +++ b/api/src/main/java/io/grpc/LoadBalancer.java @@ -124,39 +124,46 @@ public abstract class LoadBalancer { * *

Implementations should not modify the given {@code servers}. * - * @param servers the resolved server addresses, never empty. - * @param attributes extra information from naming system. - * @deprecated override {@link #handleResolvedAddresses(ResolvedAddresses) instead} - * @since 1.2.0 + * @param resolvedAddresses the resolved server addresses, attributes, and config. + * @since 1.21.0 */ - @Deprecated - public void handleResolvedAddressGroups( - List servers, - @NameResolver.ResolutionResultAttr Attributes attributes) { + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { if (recursionCount++ == 0) { - handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes).build()); + // Note that the information about the addresses actually being accepted will be lost + // if you rely on this method for backward compatibility. + acceptResolvedAddresses(resolvedAddresses); } recursionCount = 0; } /** - * Handles newly resolved server groups and metadata attributes from name resolution system. - * {@code servers} contained in {@link EquivalentAddressGroup} should be considered equivalent - * but may be flattened into a single list if needed. + * Accepts newly resolved addresses from the name resolution system. The {@link + * EquivalentAddressGroup} addresses should be considered equivalent but may be flattened into a + * single list if needed. * - *

Implementations should not modify the given {@code servers}. + *

Implementations can choose to reject the given addresses by returning {@code false}. + * + *

Implementations should not modify the given {@code addresses}. * * @param resolvedAddresses the resolved server addresses, attributes, and config. - * @since 1.21.0 + * @return {@code true} if the resolved addresses were accepted. {@code false} if rejected. + * @since 1.49.0 */ - @SuppressWarnings("deprecation") - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { - if (recursionCount++ == 0) { - handleResolvedAddressGroups( - resolvedAddresses.getAddresses(), resolvedAddresses.getAttributes()); + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + if (resolvedAddresses.getAddresses().isEmpty() + && !canHandleEmptyAddressListFromNameResolution()) { + handleNameResolutionError(Status.UNAVAILABLE.withDescription( + "NameResolver returned no usable address. addrs=" + resolvedAddresses.getAddresses() + + ", attrs=" + resolvedAddresses.getAttributes())); + return false; + } else { + if (recursionCount++ == 0) { + handleResolvedAddresses(resolvedAddresses); + } + recursionCount = 0; + + return true; } - recursionCount = 0; } /** diff --git a/api/src/test/java/io/grpc/LoadBalancerTest.java b/api/src/test/java/io/grpc/LoadBalancerTest.java index be3d10ba2a..beaf3335e2 100644 --- a/api/src/test/java/io/grpc/LoadBalancerTest.java +++ b/api/src/test/java/io/grpc/LoadBalancerTest.java @@ -235,13 +235,14 @@ public class LoadBalancerTest { @Deprecated @Test - public void handleResolvedAddressGroups_delegatesToHandleResolvedAddresses() { + public void handleResolvedAddresses_delegatesToAcceptResolvedAddresses() { final AtomicReference resultCapture = new AtomicReference<>(); LoadBalancer balancer = new LoadBalancer() { @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { resultCapture.set(resolvedAddresses); + return true; } @Override @@ -260,23 +261,22 @@ public class LoadBalancerTest { List servers = Arrays.asList( new EquivalentAddressGroup(new SocketAddress(){}), new EquivalentAddressGroup(new SocketAddress(){})); - balancer.handleResolvedAddressGroups(servers, attrs); + ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers) + .setAttributes(attrs).build(); + balancer.handleResolvedAddresses(addresses); assertThat(resultCapture.get()).isEqualTo( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attrs).build()); } @Deprecated @Test - public void handleResolvedAddresses_delegatesToHandleResolvedAddressGroups() { - final AtomicReference> serversCapture = new AtomicReference<>(); - final AtomicReference attrsCapture = new AtomicReference<>(); + public void acceptResolvedAddresses_delegatesToHandleResolvedAddressGroups() { + final AtomicReference addressesCapture = new AtomicReference<>(); LoadBalancer balancer = new LoadBalancer() { @Override - public void handleResolvedAddressGroups( - List servers, Attributes attrs) { - serversCapture.set(servers); - attrsCapture.set(attrs); + public void handleResolvedAddresses(ResolvedAddresses addresses) { + addressesCapture.set(addresses); } @Override @@ -295,25 +295,23 @@ public class LoadBalancerTest { List servers = Arrays.asList( new EquivalentAddressGroup(new SocketAddress(){}), new EquivalentAddressGroup(new SocketAddress(){})); - balancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attrs).build()); - assertThat(serversCapture.get()).isEqualTo(servers); - assertThat(attrsCapture.get()).isEqualTo(attrs); + ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers) + .setAttributes(attrs).build(); + balancer.handleResolvedAddresses(addresses); + assertThat(addressesCapture.get().getAddresses()).isEqualTo(servers); + assertThat(addressesCapture.get().getAttributes()).isEqualTo(attrs); } @Deprecated @Test - public void handleResolvedAddresses_noInfiniteLoop() { - final List> serversCapture = new ArrayList<>(); - final List attrsCapture = new ArrayList<>(); + public void acceptResolvedAddresses_noInfiniteLoop() { + final List addressesCapture = new ArrayList<>(); LoadBalancer balancer = new LoadBalancer() { @Override - public void handleResolvedAddressGroups( - List servers, Attributes attrs) { - serversCapture.add(servers); - attrsCapture.add(attrs); - super.handleResolvedAddressGroups(servers, attrs); + public void handleResolvedAddresses(ResolvedAddresses addresses) { + addressesCapture.add(addresses); + super.handleResolvedAddresses(addresses); } @Override @@ -328,12 +326,12 @@ public class LoadBalancerTest { List servers = Arrays.asList( new EquivalentAddressGroup(new SocketAddress(){}), new EquivalentAddressGroup(new SocketAddress(){})); - balancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attrs).build()); - assertThat(serversCapture).hasSize(1); - assertThat(attrsCapture).hasSize(1); - assertThat(serversCapture.get(0)).isEqualTo(servers); - assertThat(attrsCapture.get(0)).isEqualTo(attrs); + ResolvedAddresses addresses = ResolvedAddresses.newBuilder().setAddresses(servers) + .setAttributes(attrs).build(); + balancer.handleResolvedAddresses(addresses); + assertThat(addressesCapture).hasSize(1); + assertThat(addressesCapture.get(0).getAddresses()).isEqualTo(servers); + assertThat(addressesCapture.get(0).getAttributes()).isEqualTo(attrs); } private static class NoopHelper extends LoadBalancer.Helper { diff --git a/core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java b/core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java index 01c48b9efc..b8c9cab745 100644 --- a/core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java +++ b/core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java @@ -20,11 +20,9 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; -import io.grpc.Attributes; import io.grpc.ChannelLogger.ChannelLogLevel; import io.grpc.ConnectivityState; import io.grpc.ConnectivityStateInfo; -import io.grpc.EquivalentAddressGroup; import io.grpc.LoadBalancer; import io.grpc.LoadBalancer.Helper; import io.grpc.LoadBalancer.PickResult; @@ -67,10 +65,14 @@ public final class AutoConfiguredLoadBalancerFactory { @Override @Deprecated - public void handleResolvedAddressGroups(List s, Attributes a) {} + @SuppressWarnings("InlineMeSuggester") + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {} + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + return true; + } @Override public void handleNameResolutionError(Status error) {} @@ -97,14 +99,10 @@ public final class AutoConfiguredLoadBalancerFactory { } /** - * Returns non-OK status if resolvedAddresses is empty and delegate lb requires address ({@link - * LoadBalancer#canHandleEmptyAddressListFromNameResolution()} returns {@code false}). {@code - * AutoConfiguredLoadBalancer} doesn't expose {@code - * canHandleEmptyAddressListFromNameResolution} because it depends on the delegated LB. + * Returns non-OK status if the delegate rejects the resolvedAddresses (e.g. if it does not + * support an empty list). */ - Status tryHandleResolvedAddresses(ResolvedAddresses resolvedAddresses) { - List servers = resolvedAddresses.getAddresses(); - Attributes attributes = resolvedAddresses.getAttributes(); + boolean tryAcceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { PolicySelection policySelection = (PolicySelection) resolvedAddresses.getLoadBalancingPolicyConfig(); @@ -118,7 +116,7 @@ public final class AutoConfiguredLoadBalancerFactory { delegate.shutdown(); delegateProvider = null; delegate = new NoopLoadBalancer(); - return Status.OK; + return true; } policySelection = new PolicySelection(defaultProvider, /* config= */ null); @@ -141,20 +139,12 @@ public final class AutoConfiguredLoadBalancerFactory { ChannelLogLevel.DEBUG, "Load-balancing config: {0}", policySelection.config); } - LoadBalancer delegate = getDelegate(); - if (resolvedAddresses.getAddresses().isEmpty() - && !delegate.canHandleEmptyAddressListFromNameResolution()) { - return Status.UNAVAILABLE.withDescription( - "NameResolver returned no usable address. addrs=" + servers + ", attrs=" + attributes); - } else { - delegate.handleResolvedAddresses( - ResolvedAddresses.newBuilder() - .setAddresses(resolvedAddresses.getAddresses()) - .setAttributes(attributes) - .setLoadBalancingPolicyConfig(lbConfig) - .build()); - return Status.OK; - } + return getDelegate().acceptResolvedAddresses( + ResolvedAddresses.newBuilder() + .setAddresses(resolvedAddresses.getAddresses()) + .setAttributes(resolvedAddresses.getAttributes()) + .setLoadBalancingPolicyConfig(lbConfig) + .build()); } void handleNameResolutionError(Status error) { diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 3e2e3ec17f..acf0147ff0 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -1860,16 +1860,17 @@ final class ManagedChannelImpl extends ManagedChannel implements .set(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG, healthCheckingConfig) .build(); } + Attributes attributes = attrBuilder.build(); - Status handleResult = helper.lb.tryHandleResolvedAddresses( + boolean addressesAccepted = helper.lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) - .setAttributes(attrBuilder.build()) + .setAttributes(attributes) .setLoadBalancingPolicyConfig(effectiveServiceConfig.getLoadBalancingConfig()) .build()); - if (!handleResult.isOk()) { - handleErrorInSyncContext(handleResult.augmentDescription(resolver + " was used")); + if (!addressesAccepted) { + scheduleExponentialBackOffInSyncContext(); } } } diff --git a/core/src/main/java/io/grpc/internal/PickFirstLoadBalancer.java b/core/src/main/java/io/grpc/internal/PickFirstLoadBalancer.java index d5f74db54a..b105a9ed0c 100644 --- a/core/src/main/java/io/grpc/internal/PickFirstLoadBalancer.java +++ b/core/src/main/java/io/grpc/internal/PickFirstLoadBalancer.java @@ -45,7 +45,7 @@ final class PickFirstLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { List servers = resolvedAddresses.getAddresses(); if (subchannel == null) { final Subchannel subchannel = helper.createSubchannel( @@ -67,6 +67,8 @@ final class PickFirstLoadBalancer extends LoadBalancer { } else { subchannel.updateAddresses(servers); } + + return true; } @Override diff --git a/core/src/main/java/io/grpc/util/ForwardingLoadBalancer.java b/core/src/main/java/io/grpc/util/ForwardingLoadBalancer.java index 628eda3b71..65588b1181 100644 --- a/core/src/main/java/io/grpc/util/ForwardingLoadBalancer.java +++ b/core/src/main/java/io/grpc/util/ForwardingLoadBalancer.java @@ -17,14 +17,10 @@ package io.grpc.util; import com.google.common.base.MoreObjects; -import io.grpc.Attributes; import io.grpc.ConnectivityStateInfo; -import io.grpc.EquivalentAddressGroup; import io.grpc.ExperimentalApi; import io.grpc.LoadBalancer; -import io.grpc.NameResolver; import io.grpc.Status; -import java.util.List; @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1771") public abstract class ForwardingLoadBalancer extends LoadBalancer { @@ -35,15 +31,13 @@ public abstract class ForwardingLoadBalancer extends LoadBalancer { @Override @Deprecated - public void handleResolvedAddressGroups( - List servers, - @NameResolver.ResolutionResultAttr Attributes attributes) { - delegate().handleResolvedAddressGroups(servers, attributes); + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + delegate().handleResolvedAddresses(resolvedAddresses); } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { - delegate().handleResolvedAddresses(resolvedAddresses); + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + return delegate().acceptResolvedAddresses(resolvedAddresses); } @Override diff --git a/core/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java b/core/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java index 7cba3098ca..ca6d77642e 100644 --- a/core/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java +++ b/core/src/main/java/io/grpc/util/GracefulSwitchLoadBalancer.java @@ -35,7 +35,7 @@ import javax.annotation.concurrent.NotThreadSafe; * will keep using the old policy until the new policy reports READY or the old policy exits READY. * *

The balancer must {@link #switchTo(LoadBalancer.Factory) switch to} a policy prior to {@link - * LoadBalancer#handleResolvedAddresses(ResolvedAddresses) handling resolved addresses} for the + * LoadBalancer#acceptResolvedAddresses(ResolvedAddresses) handling resolved addresses} for the * first time. */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/5999") @@ -43,7 +43,7 @@ import javax.annotation.concurrent.NotThreadSafe; public final class GracefulSwitchLoadBalancer extends ForwardingLoadBalancer { private final LoadBalancer defaultBalancer = new LoadBalancer() { @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(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 diff --git a/core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java b/core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java index 4a6a1ff561..905832d44d 100644 --- a/core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java +++ b/core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java @@ -69,7 +69,7 @@ final class RoundRobinLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { List servers = resolvedAddresses.getAddresses(); Set currentAddrs = subchannels.keySet(); Map latestAddrs = stripAttrs(servers); @@ -126,6 +126,8 @@ final class RoundRobinLoadBalancer extends LoadBalancer { for (Subchannel removedSubchannel : removedSubchannels) { shutdownSubchannel(removedSubchannel); } + + return !resolvedAddresses.getAddresses().isEmpty(); } @Override diff --git a/core/src/test/java/io/grpc/internal/AutoConfiguredLoadBalancerFactoryTest.java b/core/src/test/java/io/grpc/internal/AutoConfiguredLoadBalancerFactoryTest.java index bf7c63818c..ad886c3114 100644 --- a/core/src/test/java/io/grpc/internal/AutoConfiguredLoadBalancerFactoryTest.java +++ b/core/src/test/java/io/grpc/internal/AutoConfiguredLoadBalancerFactoryTest.java @@ -21,8 +21,8 @@ import static org.junit.Assert.assertTrue; import static org.mockito.AdditionalAnswers.delegatesTo; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isA; import static org.mockito.ArgumentMatchers.same; -import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -97,9 +97,8 @@ public class AutoConfiguredLoadBalancerFactoryTest { @Before public void setUp() { - when(testLbBalancer.canHandleEmptyAddressListFromNameResolution()).thenCallRealMethod(); - assertThat(testLbBalancer.canHandleEmptyAddressListFromNameResolution()).isFalse(); - when(testLbBalancer2.canHandleEmptyAddressListFromNameResolution()).thenReturn(true); + when(testLbBalancer.acceptResolvedAddresses(isA(ResolvedAddresses.class))).thenReturn(true); + when(testLbBalancer2.acceptResolvedAddresses(isA(ResolvedAddresses.class))).thenReturn(true); defaultRegistry.register(testLbBalancerProvider); defaultRegistry.register(testLbBalancerProvider2); } @@ -171,7 +170,7 @@ public class AutoConfiguredLoadBalancerFactoryTest { } @Test - public void handleResolvedAddressGroups_keepOldBalancer() { + public void acceptResolvedAddresses_keepOldBalancer() { final List servers = Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){})); Helper helper = new TestHelper() { @@ -184,19 +183,19 @@ public class AutoConfiguredLoadBalancerFactoryTest { AutoConfiguredLoadBalancer lb = lbf.newLoadBalancer(helper); LoadBalancer oldDelegate = lb.getDelegate(); - Status handleResult = lb.tryHandleResolvedAddresses( + boolean addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setAttributes(Attributes.EMPTY) .setLoadBalancingPolicyConfig(null) .build()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); assertThat(lb.getDelegate()).isSameInstanceAs(oldDelegate); } @Test - public void handleResolvedAddressGroups_shutsDownOldBalancer() throws Exception { + public void acceptResolvedAddresses_shutsDownOldBalancer() throws Exception { Map serviceConfig = parseConfig("{\"loadBalancingConfig\": [ {\"round_robin\": { } } ] }"); ConfigOrError lbConfigs = lbf.parseLoadBalancerPolicy(serviceConfig); @@ -226,13 +225,13 @@ public class AutoConfiguredLoadBalancerFactoryTest { }; lb.setDelegate(testlb); - Status handleResult = lb.tryHandleResolvedAddresses( + boolean addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setLoadBalancingPolicyConfig(lbConfigs.getConfig()) .build()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); assertThat(lb.getDelegateProvider().getClass().getName()).isEqualTo( "io.grpc.util.SecretRoundRobinLoadBalancerProvider$Provider"); assertTrue(shutdown.get()); @@ -240,7 +239,7 @@ public class AutoConfiguredLoadBalancerFactoryTest { @Test @SuppressWarnings("unchecked") - public void handleResolvedAddressGroups_propagateLbConfigToDelegate() throws Exception { + public void acceptResolvedAddresses_propagateLbConfigToDelegate() throws Exception { Map rawServiceConfig = parseConfig("{\"loadBalancingConfig\": [ {\"test_lb\": { \"setting1\": \"high\" } } ] }"); ConfigOrError lbConfigs = lbf.parseLoadBalancerPolicy(rawServiceConfig); @@ -251,20 +250,19 @@ public class AutoConfiguredLoadBalancerFactoryTest { Helper helper = new TestHelper(); AutoConfiguredLoadBalancer lb = lbf.newLoadBalancer(helper); - Status handleResult = lb.tryHandleResolvedAddresses( + boolean addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setLoadBalancingPolicyConfig(lbConfigs.getConfig()) .build()); verify(testLbBalancerProvider).newLoadBalancer(same(helper)); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); assertThat(lb.getDelegate()).isSameInstanceAs(testLbBalancer); ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(testLbBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(testLbBalancer).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).containsExactlyElementsIn(servers).inOrder(); - verify(testLbBalancer, atLeast(0)).canHandleEmptyAddressListFromNameResolution(); ArgumentCaptor> lbConfigCaptor = ArgumentCaptor.forClass(Map.class); verify(testLbBalancerProvider).parseLoadBalancingPolicyConfig(lbConfigCaptor.capture()); assertThat(lbConfigCaptor.getValue()).containsExactly("setting1", "high"); @@ -274,7 +272,7 @@ public class AutoConfiguredLoadBalancerFactoryTest { parseConfig("{\"loadBalancingConfig\": [ {\"test_lb\": { \"setting1\": \"low\" } } ] }"); lbConfigs = lbf.parseLoadBalancerPolicy(rawServiceConfig); - handleResult = lb.tryHandleResolvedAddresses( + addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setLoadBalancingPolicyConfig(lbConfigs.getConfig()) @@ -282,8 +280,8 @@ public class AutoConfiguredLoadBalancerFactoryTest { resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(testLbBalancer, times(2)).handleResolvedAddresses(resultCaptor.capture()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + verify(testLbBalancer, times(2)).acceptResolvedAddresses(resultCaptor.capture()); + assertThat(addressesAccepted).isTrue(); assertThat(resultCaptor.getValue().getAddresses()).containsExactlyElementsIn(servers).inOrder(); verify(testLbBalancerProvider, times(2)) .parseLoadBalancingPolicyConfig(lbConfigCaptor.capture()); @@ -294,7 +292,7 @@ public class AutoConfiguredLoadBalancerFactoryTest { } @Test - public void handleResolvedAddressGroups_propagateAddrsToDelegate() throws Exception { + public void acceptResolvedAddresses_propagateAddrsToDelegate() throws Exception { Map rawServiceConfig = parseConfig("{\"loadBalancingConfig\": [ {\"test_lb\": { \"setting1\": \"high\" } } ] }"); ConfigOrError lbConfigs = lbf.parseLoadBalancerPolicy(rawServiceConfig); @@ -305,56 +303,58 @@ public class AutoConfiguredLoadBalancerFactoryTest { List servers = Collections.singletonList(new EquivalentAddressGroup(new InetSocketAddress(8080){})); - Status handleResult = lb.tryHandleResolvedAddresses( + boolean addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setLoadBalancingPolicyConfig(lbConfigs.getConfig()) .build()); verify(testLbBalancerProvider).newLoadBalancer(same(helper)); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); assertThat(lb.getDelegate()).isSameInstanceAs(testLbBalancer); ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(testLbBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(testLbBalancer).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).containsExactlyElementsIn(servers).inOrder(); servers = Collections.singletonList(new EquivalentAddressGroup(new InetSocketAddress(9090){})); - handleResult = lb.tryHandleResolvedAddresses( + addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setLoadBalancingPolicyConfig(lbConfigs.getConfig()) .build()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); - verify(testLbBalancer, times(2)).handleResolvedAddresses(resultCaptor.capture()); + assertThat(addressesAccepted).isTrue(); + verify(testLbBalancer, times(2)).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).containsExactlyElementsIn(servers).inOrder(); } @Test - public void handleResolvedAddressGroups_delegateDoNotAcceptEmptyAddressList_nothing() + public void acceptResolvedAddresses_delegateDoNotAcceptEmptyAddressList_nothing() throws Exception { + + // The test LB will NOT accept the addresses we give them. + when(testLbBalancer.acceptResolvedAddresses(isA(ResolvedAddresses.class))).thenReturn(false); + Helper helper = new TestHelper(); AutoConfiguredLoadBalancer lb = lbf.newLoadBalancer(helper); Map serviceConfig = parseConfig("{\"loadBalancingConfig\": [ {\"test_lb\": { \"setting1\": \"high\" } } ] }"); ConfigOrError lbConfig = lbf.parseLoadBalancerPolicy(serviceConfig); - Status handleResult = lb.tryHandleResolvedAddresses( + boolean addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(Collections.emptyList()) .setLoadBalancingPolicyConfig(lbConfig.getConfig()) .build()); - assertThat(testLbBalancer.canHandleEmptyAddressListFromNameResolution()).isFalse(); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.UNAVAILABLE); - assertThat(handleResult.getDescription()).startsWith("NameResolver returned no usable address"); + assertThat(addressesAccepted).isFalse(); assertThat(lb.getDelegate()).isSameInstanceAs(testLbBalancer); } @Test - public void handleResolvedAddressGroups_delegateAcceptsEmptyAddressList() + public void acceptResolvedAddresses_delegateAcceptsEmptyAddressList() throws Exception { Helper helper = new TestHelper(); AutoConfiguredLoadBalancer lb = lbf.newLoadBalancer(helper); @@ -363,25 +363,24 @@ public class AutoConfiguredLoadBalancerFactoryTest { parseConfig("{\"loadBalancingConfig\": [ {\"test_lb2\": { \"setting1\": \"high\" } } ] }"); ConfigOrError lbConfigs = lbf.parseLoadBalancerPolicy(rawServiceConfig); - Status handleResult = lb.tryHandleResolvedAddresses( + boolean addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(Collections.emptyList()) .setLoadBalancingPolicyConfig(lbConfigs.getConfig()) .build()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); assertThat(lb.getDelegate()).isSameInstanceAs(testLbBalancer2); - assertThat(testLbBalancer2.canHandleEmptyAddressListFromNameResolution()).isTrue(); ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(testLbBalancer2).handleResolvedAddresses(resultCaptor.capture()); + verify(testLbBalancer2).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).isEmpty(); assertThat(resultCaptor.getValue().getLoadBalancingPolicyConfig()) .isEqualTo(nextParsedConfigOrError2.get().getConfig()); } @Test - public void handleResolvedAddressGroups_useSelectedLbPolicy() throws Exception { + public void acceptResolvedAddresses_useSelectedLbPolicy() throws Exception { Map rawServiceConfig = parseConfig("{\"loadBalancingConfig\": [{\"round_robin\": {}}]}"); ConfigOrError lbConfigs = lbf.parseLoadBalancerPolicy(rawServiceConfig); @@ -399,18 +398,18 @@ public class AutoConfiguredLoadBalancerFactoryTest { } }; AutoConfiguredLoadBalancer lb = lbf.newLoadBalancer(helper); - Status handleResult = lb.tryHandleResolvedAddresses( + boolean addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setLoadBalancingPolicyConfig(lbConfigs.getConfig()) .build()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); assertThat(lb.getDelegate().getClass().getName()) .isEqualTo("io.grpc.util.RoundRobinLoadBalancer"); } @Test - public void handleResolvedAddressGroups_noLbPolicySelected_defaultToPickFirst() { + public void acceptResolvedAddresses_noLbPolicySelected_defaultToPickFirst() { final List servers = Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){})); Helper helper = new TestHelper() { @@ -421,27 +420,27 @@ public class AutoConfiguredLoadBalancerFactoryTest { } }; AutoConfiguredLoadBalancer lb = lbf.newLoadBalancer(helper); - Status handleResult = lb.tryHandleResolvedAddresses( + boolean addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setLoadBalancingPolicyConfig(null) .build()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); assertThat(lb.getDelegate()).isInstanceOf(PickFirstLoadBalancer.class); } @Test - public void handleResolvedAddressGroups_noLbPolicySelected_defaultToCustomDefault() { + public void acceptResolvedAddresses_noLbPolicySelected_defaultToCustomDefault() { AutoConfiguredLoadBalancer lb = new AutoConfiguredLoadBalancerFactory("test_lb") .newLoadBalancer(new TestHelper()); List servers = Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){})); - Status handleResult = lb.tryHandleResolvedAddresses( + boolean addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setLoadBalancingPolicyConfig(null) .build()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); assertThat(lb.getDelegate()).isSameInstanceAs(testLbBalancer); } @@ -458,13 +457,13 @@ public class AutoConfiguredLoadBalancerFactoryTest { AutoConfiguredLoadBalancer lb = new AutoConfiguredLoadBalancerFactory(GrpcUtil.DEFAULT_LB_POLICY).newLoadBalancer(helper); - Status handleResult = lb.tryHandleResolvedAddresses( + boolean addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setAttributes(Attributes.EMPTY) .build()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); verifyNoMoreInteractions(channelLogger); ConfigOrError testLbParsedConfig = ConfigOrError.fromConfig("foo"); @@ -472,13 +471,13 @@ public class AutoConfiguredLoadBalancerFactoryTest { Map serviceConfig = parseConfig("{\"loadBalancingConfig\": [ {\"test_lb\": { } } ] }"); ConfigOrError lbConfigs = lbf.parseLoadBalancerPolicy(serviceConfig); - handleResult = lb.tryHandleResolvedAddresses( + addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setLoadBalancingPolicyConfig(lbConfigs.getConfig()) .build()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); verify(channelLogger).log( eq(ChannelLogLevel.INFO), eq("Load balancer changed from {0} to {1}"), @@ -495,12 +494,12 @@ public class AutoConfiguredLoadBalancerFactoryTest { nextParsedConfigOrError.set(testLbParsedConfig); serviceConfig = parseConfig("{\"loadBalancingConfig\": [ {\"test_lb\": { } } ] }"); lbConfigs = lbf.parseLoadBalancerPolicy(serviceConfig); - handleResult = lb.tryHandleResolvedAddresses( + addressesAccepted = lb.tryAcceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers) .setLoadBalancingPolicyConfig(lbConfigs.getConfig()) .build()); - assertThat(handleResult.getCode()).isEqualTo(Status.Code.OK); + assertThat(addressesAccepted).isTrue(); verify(channelLogger).log( eq(ChannelLogLevel.DEBUG), eq("Load-balancing config: {0}"), @@ -643,14 +642,13 @@ public class AutoConfiguredLoadBalancerFactoryTest { @Override @Deprecated - public void handleResolvedAddressGroups( - List servers, Attributes attributes) { - delegate().handleResolvedAddressGroups(servers, attributes); + public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + delegate().acceptResolvedAddresses(resolvedAddresses); } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { - delegate().handleResolvedAddresses(resolvedAddresses); + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + return delegate().acceptResolvedAddresses(resolvedAddresses); } @Override diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java index 30e137cba2..90d2d2d8d1 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java @@ -154,6 +154,7 @@ public class ManagedChannelImplIdlenessTest { @Before @SuppressWarnings("deprecation") // For NameResolver.Listener public void setUp() { + when(mockLoadBalancer.acceptResolvedAddresses(isA(ResolvedAddresses.class))).thenReturn(true); LoadBalancerRegistry.getDefaultRegistry().register(mockLoadBalancerProvider); when(mockNameResolver.getServiceAuthority()).thenReturn(AUTHORITY); when(mockNameResolverFactory @@ -220,7 +221,7 @@ public class ManagedChannelImplIdlenessTest { ArgumentCaptor resolvedAddressCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resolvedAddressCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resolvedAddressCaptor.capture()); assertThat(resolvedAddressCaptor.getValue().getAddresses()) .containsExactlyElementsIn(servers); } diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index b5c8ca0c98..b229e951fb 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -327,7 +327,7 @@ public class ManagedChannelImplTest { @Before public void setUp() throws Exception { - when(mockLoadBalancer.canHandleEmptyAddressListFromNameResolution()).thenCallRealMethod(); + when(mockLoadBalancer.acceptResolvedAddresses(isA(ResolvedAddresses.class))).thenReturn(true); LoadBalancerRegistry.getDefaultRegistry().register(mockLoadBalancerProvider); expectedUri = new URI(TARGET); transports = TestUtils.captureTransports(mockTransportFactory); @@ -948,7 +948,7 @@ public class ManagedChannelImplTest { FakeNameResolverFactory.FakeNameResolver resolver = nameResolverFactory.resolvers.get(0); verify(mockLoadBalancerProvider).newLoadBalancer(any(Helper.class)); - verify(mockLoadBalancer).handleResolvedAddresses(resolvedAddressCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resolvedAddressCaptor.capture()); assertThat(resolvedAddressCaptor.getValue().getAddresses()).containsExactly(addressGroup); SubchannelStateListener stateListener1 = mock(SubchannelStateListener.class); @@ -1163,8 +1163,9 @@ public class ManagedChannelImplTest { } @Test - public void nameResolverReturnsEmptySubLists_becomeErrorByDefault() throws Exception { - String errorDescription = "NameResolver returned no usable address"; + public void nameResolverReturnsEmptySubLists_resolutionRetry() throws Exception { + // The mock LB is set to reject the addresses. + when(mockLoadBalancer.acceptResolvedAddresses(isA(ResolvedAddresses.class))).thenReturn(false); // Pass a FakeNameResolverFactory with an empty list and LB config FakeNameResolverFactory nameResolverFactory = @@ -1177,21 +1178,12 @@ public class ManagedChannelImplTest { channelBuilder.nameResolverFactory(nameResolverFactory); createChannel(); - // LoadBalancer received the error - verify(mockLoadBalancerProvider).newLoadBalancer(any(Helper.class)); - verify(mockLoadBalancer).handleNameResolutionError(statusCaptor.capture()); - Status status = statusCaptor.getValue(); - assertSame(Status.Code.UNAVAILABLE, status.getCode()); - assertThat(status.getDescription()).startsWith(errorDescription); - // A resolution retry has been scheduled assertEquals(1, timer.numPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER)); } @Test public void nameResolverReturnsEmptySubLists_optionallyAllowed() throws Exception { - when(mockLoadBalancer.canHandleEmptyAddressListFromNameResolution()).thenReturn(true); - // Pass a FakeNameResolverFactory with an empty list and LB config FakeNameResolverFactory nameResolverFactory = new FakeNameResolverFactory.Builder(expectedUri).build(); @@ -1213,7 +1205,7 @@ public class ManagedChannelImplTest { verify(mockLoadBalancerProvider).newLoadBalancer(any(Helper.class)); ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).isEmpty(); assertThat(resultCaptor.getValue().getLoadBalancingPolicyConfig()).isEqualTo(parsedLbConfig); @@ -1234,7 +1226,7 @@ public class ManagedChannelImplTest { createChannel(); verify(mockLoadBalancerProvider).newLoadBalancer(any(Helper.class)); - doThrow(ex).when(mockLoadBalancer).handleResolvedAddresses(any(ResolvedAddresses.class)); + doThrow(ex).when(mockLoadBalancer).acceptResolvedAddresses(any(ResolvedAddresses.class)); // NameResolver returns addresses. nameResolverFactory.allResolved(); @@ -1296,7 +1288,7 @@ public class ManagedChannelImplTest { // Simulate name resolution results EquivalentAddressGroup addressGroup = new EquivalentAddressGroup(resolvedAddrs); - inOrder.verify(mockLoadBalancer).handleResolvedAddresses(resolvedAddressCaptor.capture()); + inOrder.verify(mockLoadBalancer).acceptResolvedAddresses(resolvedAddressCaptor.capture()); assertThat(resolvedAddressCaptor.getValue().getAddresses()).containsExactly(addressGroup); Subchannel subchannel = createSubchannelSafely(helper, addressGroup, Attributes.EMPTY, subchannelStateListener); @@ -1446,7 +1438,7 @@ public class ManagedChannelImplTest { // Simulate name resolution results EquivalentAddressGroup addressGroup = new EquivalentAddressGroup(resolvedAddrs); - inOrder.verify(mockLoadBalancer).handleResolvedAddresses(resolvedAddressCaptor.capture()); + inOrder.verify(mockLoadBalancer).acceptResolvedAddresses(resolvedAddressCaptor.capture()); assertThat(resolvedAddressCaptor.getValue().getAddresses()).containsExactly(addressGroup); Subchannel subchannel = @@ -3772,7 +3764,7 @@ public class ManagedChannelImplTest { ArgumentCaptor helperCaptor = ArgumentCaptor.forClass(Helper.class); verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture()); helper = helperCaptor.getValue(); - verify(mockLoadBalancer).handleResolvedAddresses( + verify(mockLoadBalancer).acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(nameResolverFactory.servers) .build()); @@ -3878,7 +3870,7 @@ public class ManagedChannelImplTest { ArgumentCaptor helperCaptor = ArgumentCaptor.forClass(Helper.class); verify(mockLoadBalancerProvider).newLoadBalancer(helperCaptor.capture()); helper = helperCaptor.getValue(); - verify(mockLoadBalancer).handleResolvedAddresses( + verify(mockLoadBalancer).acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(nameResolverFactory.servers) .build()); @@ -4205,7 +4197,7 @@ public class ManagedChannelImplTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).containsExactly(addressGroup); assertThat(resultCaptor.getValue().getAttributes().get(InternalConfigSelector.KEY)).isNull(); verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); @@ -4243,7 +4235,7 @@ public class ManagedChannelImplTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).containsExactly(addressGroup); assertThat(resultCaptor.getValue().getAttributes().get(InternalConfigSelector.KEY)).isNull(); verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); @@ -4273,7 +4265,7 @@ public class ManagedChannelImplTest { createChannel(); ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).containsExactly(addressGroup); verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); @@ -4289,7 +4281,7 @@ public class ManagedChannelImplTest { nameResolverFactory.allResolved(); resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer, times(2)).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer, times(2)).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).containsExactly(addressGroup); verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); } finally { @@ -4323,7 +4315,7 @@ public class ManagedChannelImplTest { createChannel(); ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).containsExactly(addressGroup); verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); } finally { @@ -4351,7 +4343,7 @@ public class ManagedChannelImplTest { createChannel(); ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).containsExactly(addressGroup); verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); } finally { @@ -4377,7 +4369,7 @@ public class ManagedChannelImplTest { createChannel(); ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAddresses()).containsExactly(addressGroup); verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); } finally { @@ -4457,7 +4449,7 @@ public class ManagedChannelImplTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); assertThat(resultCaptor.getValue().getAttributes() .get(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG)) .containsExactly("serviceName", "service1"); diff --git a/core/src/test/java/io/grpc/internal/PickFirstLoadBalancerTest.java b/core/src/test/java/io/grpc/internal/PickFirstLoadBalancerTest.java index 720341da0c..1cb4c15b07 100644 --- a/core/src/test/java/io/grpc/internal/PickFirstLoadBalancerTest.java +++ b/core/src/test/java/io/grpc/internal/PickFirstLoadBalancerTest.java @@ -121,7 +121,7 @@ public class PickFirstLoadBalancerTest { @Test public void pickAfterResolved() throws Exception { - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); verify(mockHelper).createSubchannel(createArgsCaptor.capture()); @@ -139,7 +139,7 @@ public class PickFirstLoadBalancerTest { @Test public void requestConnectionPicker() throws Exception { - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); InOrder inOrder = inOrder(mockHelper, mockSubchannel); @@ -164,7 +164,7 @@ public class PickFirstLoadBalancerTest { @Test public void refreshNameResolutionAfterSubchannelConnectionBroken() { - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); verify(mockHelper).createSubchannel(any(CreateSubchannelArgs.class)); @@ -196,11 +196,11 @@ public class PickFirstLoadBalancerTest { @Test public void pickAfterResolvedAndUnchanged() throws Exception { - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); verify(mockSubchannel).start(any(SubchannelStateListener.class)); verify(mockSubchannel).requestConnection(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); verify(mockSubchannel).updateAddresses(eq(servers)); verifyNoMoreInteractions(mockSubchannel); @@ -223,7 +223,7 @@ public class PickFirstLoadBalancerTest { InOrder inOrder = inOrder(mockHelper, mockSubchannel); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); inOrder.verify(mockHelper).createSubchannel(createArgsCaptor.capture()); verify(mockSubchannel).start(any(SubchannelStateListener.class)); @@ -233,7 +233,7 @@ public class PickFirstLoadBalancerTest { verify(mockSubchannel).requestConnection(); assertEquals(mockSubchannel, pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel()); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(newServers).setAttributes(affinity).build()); inOrder.verify(mockSubchannel).updateAddresses(eq(newServers)); @@ -245,7 +245,7 @@ public class PickFirstLoadBalancerTest { public void pickAfterStateChangeAfterResolution() throws Exception { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); inOrder.verify(mockHelper).createSubchannel(createArgsCaptor.capture()); CreateSubchannelArgs args = createArgsCaptor.getValue(); @@ -297,7 +297,7 @@ public class PickFirstLoadBalancerTest { .updateBalancingState(any(ConnectivityState.class), any(SubchannelPicker.class)); verify(mockSubchannel, never()).requestConnection(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); inOrder.verify(mockHelper).createSubchannel(createArgsCaptor.capture()); CreateSubchannelArgs args = createArgsCaptor.getValue(); @@ -318,7 +318,7 @@ public class PickFirstLoadBalancerTest { @Test public void nameResolutionErrorWithStateChanges() throws Exception { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); inOrder.verify(mockHelper).createSubchannel(createArgsCaptor.capture()); verify(mockSubchannel).start(stateListenerCaptor.capture()); @@ -358,7 +358,7 @@ public class PickFirstLoadBalancerTest { loadBalancer.requestConnection(); verify(mockSubchannel, never()).requestConnection(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); verify(mockSubchannel).requestConnection(); diff --git a/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java b/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java index 16c6f3bf30..1f90606042 100644 --- a/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java +++ b/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java @@ -192,7 +192,7 @@ public class ServiceConfigErrorHandlingTest { @Before public void setUp() throws Exception { - when(mockLoadBalancer.canHandleEmptyAddressListFromNameResolution()).thenCallRealMethod(); + mockLoadBalancer.setAcceptAddresses(true); LoadBalancerRegistry.getDefaultRegistry().register(mockLoadBalancerProvider); expectedUri = new URI(TARGET); when(mockTransportFactory.getScheduledExecutorService()) @@ -268,7 +268,7 @@ public class ServiceConfigErrorHandlingTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); ResolvedAddresses resolvedAddresses = resultCaptor.getValue(); assertThat(resolvedAddresses.getAddresses()).containsExactly(addressGroup); assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo("12"); @@ -280,19 +280,14 @@ public class ServiceConfigErrorHandlingTest { nameResolverFactory.servers.clear(); // 2nd resolution + mockLoadBalancer.setAcceptAddresses(false); nameResolverFactory.allResolved(); // 2nd service config without addresses - ArgumentCaptor statusCaptor = ArgumentCaptor.forClass(Status.class); - verify(mockLoadBalancer, never()).handleResolvedAddresses(any(ResolvedAddresses.class)); - verify(mockLoadBalancer).handleNameResolutionError(statusCaptor.capture()); - assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.UNAVAILABLE); - assertThat(statusCaptor.getValue().getDescription()) - .contains("NameResolver returned no usable address."); - assertThat(channel.getState(true)).isEqualTo(ConnectivityState.TRANSIENT_FAILURE); - assertWithMessage("Empty address should schedule NameResolver retry") - .that(getNameResolverRefresh()) - .isNotNull(); + verify(mockLoadBalancer).acceptResolvedAddresses(any(ResolvedAddresses.class)); + + // A resolution retry has been scheduled + assertEquals(1, timer.numPendingTasks(NAME_RESOLVER_REFRESH_TASK_FILTER)); } @Test @@ -302,7 +297,6 @@ public class ServiceConfigErrorHandlingTest { .setServers(Collections.emptyList()) .build(); channelBuilder.nameResolverFactory(nameResolverFactory); - when(mockLoadBalancer.canHandleEmptyAddressListFromNameResolution()).thenReturn(true); Map rawServiceConfig = parseJson("{\"loadBalancingConfig\": [{\"mock_lb\": {\"check\": \"val\"}}]}"); @@ -312,11 +306,11 @@ public class ServiceConfigErrorHandlingTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); ResolvedAddresses resolvedAddresses = resultCaptor.getValue(); assertThat(resolvedAddresses.getAddresses()).isEmpty(); - assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo("val");; + assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo("val"); verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); assertThat(channel.getState(false)).isNotEqualTo(ConnectivityState.TRANSIENT_FAILURE); @@ -338,7 +332,7 @@ public class ServiceConfigErrorHandlingTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); ResolvedAddresses resolvedAddresses = resultCaptor.getValue(); assertThat(resolvedAddresses.getAddresses()).containsExactly(addressGroup); assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo("foo"); @@ -360,7 +354,7 @@ public class ServiceConfigErrorHandlingTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); ResolvedAddresses resolvedAddresses = resultCaptor.getValue(); assertThat(resolvedAddresses.getAddresses()).containsExactly(addressGroup); assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isNull(); @@ -386,7 +380,7 @@ public class ServiceConfigErrorHandlingTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); ResolvedAddresses resolvedAddresses = resultCaptor.getValue(); assertThat(resolvedAddresses.getAddresses()).containsExactly(addressGroup); assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo("foo"); @@ -431,7 +425,7 @@ public class ServiceConfigErrorHandlingTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); ResolvedAddresses resolvedAddresses = resultCaptor.getValue(); assertThat(resolvedAddresses.getAddresses()).containsExactly(addressGroup); @@ -462,7 +456,7 @@ public class ServiceConfigErrorHandlingTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); ResolvedAddresses resolvedAddresses = resultCaptor.getValue(); assertThat(resolvedAddresses.getAddresses()).containsExactly(addressGroup); assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo("1st raw config"); @@ -477,7 +471,7 @@ public class ServiceConfigErrorHandlingTest { nextLbPolicyConfigError.set(Status.UNKNOWN); nameResolverFactory.allResolved(); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); ResolvedAddresses newResolvedAddress = resultCaptor.getValue(); // should use previous service config because new service config is invalid. assertThat(newResolvedAddress.getLoadBalancingPolicyConfig()).isEqualTo("1st raw config"); @@ -510,7 +504,7 @@ public class ServiceConfigErrorHandlingTest { ArgumentCaptor resultCaptor = ArgumentCaptor.forClass(ResolvedAddresses.class); - verify(mockLoadBalancer).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer).acceptResolvedAddresses(resultCaptor.capture()); ResolvedAddresses resolvedAddresses = resultCaptor.getValue(); assertThat(resolvedAddresses.getAddresses()).containsExactly(addressGroup); // should use previous service config because new resolution result is no config. @@ -522,7 +516,7 @@ public class ServiceConfigErrorHandlingTest { nameResolverFactory.nextRawServiceConfig.set(null); nameResolverFactory.allResolved(); - verify(mockLoadBalancer, times(2)).handleResolvedAddresses(resultCaptor.capture()); + verify(mockLoadBalancer, times(2)).acceptResolvedAddresses(resultCaptor.capture()); ResolvedAddresses newResolvedAddress = resultCaptor.getValue(); assertThat(newResolvedAddress.getLoadBalancingPolicyConfig()).isEqualTo("mate"); assertThat(newResolvedAddress.getAttributes().get(InternalConfigSelector.KEY)) @@ -658,6 +652,8 @@ public class ServiceConfigErrorHandlingTest { private static class FakeLoadBalancer extends LoadBalancer { + private boolean acceptAddresses = true; + @Nullable private Helper helper; @@ -665,6 +661,15 @@ public class ServiceConfigErrorHandlingTest { this.helper = helper; } + @Override + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { + return acceptAddresses; + } + + public void setAcceptAddresses(boolean acceptAddresses) { + this.acceptAddresses = acceptAddresses; + } + @Override public void handleNameResolutionError(final Status error) { helper.updateBalancingState(ConnectivityState.TRANSIENT_FAILURE, diff --git a/core/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java b/core/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java index 2131cd725b..b427f67952 100644 --- a/core/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java +++ b/core/src/test/java/io/grpc/util/GracefulSwitchLoadBalancerTest.java @@ -22,7 +22,6 @@ 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.eq; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -86,35 +85,6 @@ public class GracefulSwitchLoadBalancerTest { } } - @Test - public void canHandleEmptyAddressListFromNameResolutionForwardedToLatestPolicy() { - gracefulSwitchLb.switchTo(lbProviders.get(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(); - doReturn(true).when(lb0).canHandleEmptyAddressListFromNameResolution(); - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isTrue(); - - gracefulSwitchLb.switchTo(lbProviders.get(lbPolicies[1])); - LoadBalancer lb1 = balancers.get(lbPolicies[1]); - - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isFalse(); - - doReturn(true).when(lb1).canHandleEmptyAddressListFromNameResolution(); - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isTrue(); - - gracefulSwitchLb.switchTo(lbProviders.get(lbPolicies[2])); - LoadBalancer lb2 = balancers.get(lbPolicies[2]); - - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isFalse(); - - doReturn(true).when(lb2).canHandleEmptyAddressListFromNameResolution(); - assertThat(gracefulSwitchLb.canHandleEmptyAddressListFromNameResolution()).isTrue(); - } - @Test public void handleResolvedAddressesAndNameResolutionErrorForwardedToLatestPolicy() { gracefulSwitchLb.switchTo(lbProviders.get(lbPolicies[0])); @@ -124,17 +94,17 @@ public class GracefulSwitchLoadBalancerTest { helper0.updateBalancingState(READY, picker); ResolvedAddresses addresses = newFakeAddresses(); - gracefulSwitchLb.handleResolvedAddresses(addresses); - verify(lb0).handleResolvedAddresses(addresses); + gracefulSwitchLb.acceptResolvedAddresses(addresses); + verify(lb0).acceptResolvedAddresses(addresses); gracefulSwitchLb.handleNameResolutionError(Status.DATA_LOSS); verify(lb0).handleNameResolutionError(Status.DATA_LOSS); gracefulSwitchLb.switchTo(lbProviders.get(lbPolicies[1])); LoadBalancer lb1 = balancers.get(lbPolicies[1]); addresses = newFakeAddresses(); - gracefulSwitchLb.handleResolvedAddresses(addresses); - verify(lb0, never()).handleResolvedAddresses(addresses); - verify(lb1).handleResolvedAddresses(addresses); + 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); @@ -143,10 +113,10 @@ public class GracefulSwitchLoadBalancerTest { 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.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); diff --git a/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java b/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java index 35b04d5982..42f4b5cf75 100644 --- a/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java +++ b/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java @@ -148,7 +148,7 @@ public class RoundRobinLoadBalancerTest { @Test public void pickAfterResolved() throws Exception { final Subchannel readySubchannel = subchannels.values().iterator().next(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); deliverSubchannelState(readySubchannel, ConnectivityStateInfo.forNonError(READY)); @@ -199,7 +199,7 @@ public class RoundRobinLoadBalancerTest { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(currentServers).setAttributes(affinity) .build()); @@ -221,7 +221,7 @@ public class RoundRobinLoadBalancerTest { // This time with Attributes List latestServers = Lists.newArrayList(oldEag2, newEag); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(latestServers).setAttributes(affinity).build()); verify(newSubchannel, times(1)).requestConnection(); @@ -241,7 +241,7 @@ public class RoundRobinLoadBalancerTest { assertThat(getList(picker)).containsExactly(oldSubchannel, newSubchannel); // test going from non-empty to empty - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(Collections.emptyList()) .setAttributes(affinity) @@ -256,7 +256,7 @@ public class RoundRobinLoadBalancerTest { @Test public void pickAfterStateChange() throws Exception { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .build()); Subchannel subchannel = loadBalancer.getSubchannels().iterator().next(); @@ -296,7 +296,7 @@ public class RoundRobinLoadBalancerTest { @Test public void ignoreShutdownSubchannelStateChange() { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .build()); inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class)); @@ -315,7 +315,7 @@ public class RoundRobinLoadBalancerTest { @Test public void stayTransientFailureUntilReady() { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .build()); @@ -353,7 +353,7 @@ public class RoundRobinLoadBalancerTest { @Test public void refreshNameResolutionWhenSubchannelConnectionBroken() { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .build()); @@ -420,7 +420,7 @@ public class RoundRobinLoadBalancerTest { @Test public void nameResolutionErrorWithActiveChannels() throws Exception { final Subchannel readySubchannel = subchannels.values().iterator().next(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); deliverSubchannelState(readySubchannel, ConnectivityStateInfo.forNonError(READY)); loadBalancer.handleNameResolutionError(Status.NOT_FOUND.withDescription("nameResolutionError")); @@ -449,7 +449,7 @@ public class RoundRobinLoadBalancerTest { Subchannel sc2 = subchannelIterator.next(); Subchannel sc3 = subchannelIterator.next(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .build()); verify(sc1, times(1)).requestConnection(); diff --git a/grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancer.java b/grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancer.java index 65293d2451..bc608d66d9 100644 --- a/grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancer.java +++ b/grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancer.java @@ -76,7 +76,7 @@ class GrpclbLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { Attributes attributes = resolvedAddresses.getAttributes(); List newLbAddresses = attributes.get(GrpclbConstants.ATTR_LB_ADDRS); if (newLbAddresses == null) { @@ -85,7 +85,7 @@ class GrpclbLoadBalancer extends LoadBalancer { if (newLbAddresses.isEmpty() && resolvedAddresses.getAddresses().isEmpty()) { handleNameResolutionError( Status.UNAVAILABLE.withDescription("No backend or balancer addresses found")); - return; + return false; } List overrideAuthorityLbAddresses = new ArrayList<>(newLbAddresses.size()); @@ -114,6 +114,8 @@ class GrpclbLoadBalancer extends LoadBalancer { } grpclbState.handleAddresses(Collections.unmodifiableList(overrideAuthorityLbAddresses), newBackendServers); + + return true; } @Override @@ -150,11 +152,6 @@ class GrpclbLoadBalancer extends LoadBalancer { } } - @Override - public boolean canHandleEmptyAddressListFromNameResolution() { - return true; - } - @VisibleForTesting @Nullable GrpclbState getGrpclbState() { diff --git a/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java b/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java index 54ec832297..66fd850802 100644 --- a/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java +++ b/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java @@ -2735,7 +2735,7 @@ public class GrpclbLoadBalancerTest { syncContext.execute(new Runnable() { @Override public void run() { - balancer.handleResolvedAddresses( + balancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(backendAddrs) .setAttributes(attrs) diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProvider.java b/interop-testing/src/main/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProvider.java index 212ea7bed1..629b681916 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProvider.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProvider.java @@ -111,10 +111,12 @@ public class RpcBehaviorLoadBalancerProvider extends LoadBalancerProvider { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { helper.setRpcBehavior( ((RpcBehaviorConfig) resolvedAddresses.getLoadBalancingPolicyConfig()).rpcBehavior); - delegateLb.handleResolvedAddresses(resolvedAddresses); + delegateLb.acceptResolvedAddresses(resolvedAddresses); + + return true; } } diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProviderTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProviderTest.java index e19208b888..d2929cc285 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProviderTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/RpcBehaviorLoadBalancerProviderTest.java @@ -83,8 +83,8 @@ public class RpcBehaviorLoadBalancerProviderTest { RpcBehaviorLoadBalancer lb = new RpcBehaviorLoadBalancer(new RpcBehaviorHelper(mockHelper), mockDelegateLb); ResolvedAddresses resolvedAddresses = buildResolvedAddresses(buildConfig()); - lb.handleResolvedAddresses(resolvedAddresses); - verify(mockDelegateLb).handleResolvedAddresses(resolvedAddresses); + lb.acceptResolvedAddresses(resolvedAddresses); + verify(mockDelegateLb).acceptResolvedAddresses(resolvedAddresses); } @Test diff --git a/rls/src/main/java/io/grpc/rls/LbPolicyConfiguration.java b/rls/src/main/java/io/grpc/rls/LbPolicyConfiguration.java index 0abef014f8..e4c35d0d63 100644 --- a/rls/src/main/java/io/grpc/rls/LbPolicyConfiguration.java +++ b/rls/src/main/java/io/grpc/rls/LbPolicyConfiguration.java @@ -302,7 +302,7 @@ final class LbPolicyConfiguration { new Runnable() { @Override public void run() { - lb.handleResolvedAddresses( + lb.acceptResolvedAddresses( childLbResolvedAddressFactory.create(lbConfig.getConfig())); lb.requestConnection(); } diff --git a/rls/src/main/java/io/grpc/rls/RlsLoadBalancer.java b/rls/src/main/java/io/grpc/rls/RlsLoadBalancer.java index 2aac96cadc..cea1bda45d 100644 --- a/rls/src/main/java/io/grpc/rls/RlsLoadBalancer.java +++ b/rls/src/main/java/io/grpc/rls/RlsLoadBalancer.java @@ -49,7 +49,7 @@ final class RlsLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { logger.log(ChannelLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses); LbPolicyConfiguration lbPolicyConfiguration = (LbPolicyConfiguration) resolvedAddresses.getLoadBalancingPolicyConfig(); @@ -78,6 +78,8 @@ final class RlsLoadBalancer extends LoadBalancer { // not required. this.lbPolicyConfiguration = lbPolicyConfiguration; } + + return true; } @Override diff --git a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java index 69aa27af18..53eaf21f9c 100644 --- a/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java +++ b/rls/src/test/java/io/grpc/rls/CachingRlsLbClientTest.java @@ -566,7 +566,7 @@ public class CachingRlsLbClientTest { LoadBalancer loadBalancer = new LoadBalancer() { @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { Map config = (Map) resolvedAddresses.getLoadBalancingPolicyConfig(); if (DEFAULT_TARGET.equals(config.get("target"))) { helper.updateBalancingState( @@ -588,6 +588,8 @@ public class CachingRlsLbClientTest { } }); } + + return true; } @Override diff --git a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java index def99eb6ae..ca1fa96ba8 100644 --- a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java +++ b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java @@ -412,7 +412,7 @@ public class RlsLoadBalancerTest { ConfigOrError parsedConfigOrError = provider.parseLoadBalancingPolicyConfig(getServiceConfig()); assertThat(parsedConfigOrError.getConfig()).isNotNull(); - rlsLb.handleResolvedAddresses(ResolvedAddresses.newBuilder() + rlsLb.acceptResolvedAddresses(ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of(new EquivalentAddressGroup(mock(SocketAddress.class)))) .setLoadBalancingPolicyConfig(parsedConfigOrError.getConfig()) .build()); diff --git a/services/src/main/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactory.java b/services/src/main/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactory.java index d027d13f08..0d6148f6a1 100644 --- a/services/src/main/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactory.java +++ b/services/src/main/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactory.java @@ -179,14 +179,14 @@ final class HealthCheckingLoadBalancerFactory extends LoadBalancer.Factory { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { Map healthCheckingConfig = resolvedAddresses .getAttributes() .get(LoadBalancer.ATTR_HEALTH_CHECKING_CONFIG); String serviceName = ServiceConfigUtil.getHealthCheckedServiceName(healthCheckingConfig); helper.setHealthCheckedService(serviceName); - super.handleResolvedAddresses(resolvedAddresses); + return super.acceptResolvedAddresses(resolvedAddresses); } @Override diff --git a/services/src/test/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactoryTest.java b/services/src/test/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactoryTest.java index c1e11941a4..765cf1051a 100644 --- a/services/src/test/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactoryTest.java +++ b/services/src/test/java/io/grpc/protobuf/services/HealthCheckingLoadBalancerFactoryTest.java @@ -193,15 +193,16 @@ public class HealthCheckingLoadBalancerFactoryTest { boolean shutdown; @Override - public void handleResolvedAddresses(final ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(final ResolvedAddresses resolvedAddresses) { syncContext.execute(new Runnable() { @Override public void run() { if (!shutdown) { - hcLb.handleResolvedAddresses(resolvedAddresses); + hcLb.acceptResolvedAddresses(resolvedAddresses); } } }); + return true; } @Override @@ -251,9 +252,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verify(origHelper, atLeast(0)).getSynchronizationContext(); verify(origHelper, atLeast(0)).getScheduledExecutorService(); verifyNoMoreInteractions(origHelper); @@ -372,9 +373,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verifyNoMoreInteractions(origLb); // We create 2 Subchannels. One of them connects to a server that doesn't implement health check @@ -441,9 +442,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verifyNoMoreInteractions(origLb); FakeSubchannel subchannel = unwrap(createSubchannel(0, Attributes.EMPTY)); @@ -512,9 +513,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verifyNoMoreInteractions(origLb); SubchannelStateListener mockStateListener = mockStateListeners[0]; @@ -605,9 +606,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(Attributes.EMPTY) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); // First, create Subchannels 0 @@ -626,8 +627,8 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); - verify(origLb).handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); + verify(origLb).acceptResolvedAddresses(result2); // Health check started on existing Subchannel assertThat(healthImpls[0].calls).hasSize(1); @@ -649,9 +650,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); Subchannel subchannel = createSubchannel(0, Attributes.EMPTY); @@ -672,7 +673,7 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(Attributes.EMPTY) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); // Health check RPC cancelled. assertThat(serverCall.cancelled).isTrue(); @@ -680,7 +681,7 @@ public class HealthCheckingLoadBalancerFactoryTest { inOrder.verify(mockStateListeners[0]).onSubchannelState( eq(ConnectivityStateInfo.forNonError(READY))); - inOrder.verify(origLb).handleResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); verifyNoMoreInteractions(origLb, mockStateListeners[0]); assertThat(healthImpl.calls).isEmpty(); @@ -693,9 +694,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verifyNoMoreInteractions(origLb); Subchannel subchannel = createSubchannel(0, Attributes.EMPTY); @@ -722,7 +723,7 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(Attributes.EMPTY) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); // Retry timer is cancelled assertThat(clock.getPendingTasks()).isEmpty(); @@ -734,7 +735,7 @@ public class HealthCheckingLoadBalancerFactoryTest { inOrder.verify(mockStateListeners[0]).onSubchannelState( eq(ConnectivityStateInfo.forNonError(READY))); - inOrder.verify(origLb).handleResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); verifyNoMoreInteractions(origLb, mockStateListeners[0]); } @@ -746,9 +747,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); Subchannel subchannel = createSubchannel(0, Attributes.EMPTY); @@ -768,9 +769,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(Attributes.EMPTY) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); - inOrder.verify(origLb).handleResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); // Underlying subchannel is now ready deliverSubchannelState(0, ConnectivityStateInfo.forNonError(READY)); @@ -792,9 +793,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); Subchannel subchannel = createSubchannel(0, Attributes.EMPTY); @@ -818,9 +819,9 @@ public class HealthCheckingLoadBalancerFactoryTest { eq(ConnectivityStateInfo.forNonError(READY))); // Service config returns with the same health check name. - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); // It's delivered to origLb, but nothing else happens - inOrder.verify(origLb).handleResolvedAddresses(result1); + inOrder.verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb, mockListener); // Service config returns a different health check name. @@ -829,8 +830,8 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); - inOrder.verify(origLb).handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); // Current health check RPC cancelled. assertThat(serverCall.cancelled).isTrue(); @@ -852,9 +853,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); Subchannel subchannel = createSubchannel(0, Attributes.EMPTY); @@ -883,9 +884,9 @@ public class HealthCheckingLoadBalancerFactoryTest { // Service config returns with the same health check name. - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); // It's delivered to origLb, but nothing else happens - inOrder.verify(origLb).handleResolvedAddresses(result1); + inOrder.verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb, mockListener); assertThat(clock.getPendingTasks()).hasSize(1); assertThat(healthImpl.calls).isEmpty(); @@ -896,13 +897,13 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); // Concluded CONNECTING state inOrder.verify(mockListener).onSubchannelState( eq(ConnectivityStateInfo.forNonError(CONNECTING))); - inOrder.verify(origLb).handleResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); // Current retry timer cancelled assertThat(clock.getPendingTasks()).isEmpty(); @@ -923,9 +924,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); - verify(origLb).handleResolvedAddresses(result1); + verify(origLb).acceptResolvedAddresses(result1); verifyNoMoreInteractions(origLb); Subchannel subchannel = createSubchannel(0, Attributes.EMPTY); @@ -943,9 +944,9 @@ public class HealthCheckingLoadBalancerFactoryTest { inOrder.verifyNoMoreInteractions(); // Service config returns with the same health check name. - hcLbEventDelivery.handleResolvedAddresses(result1); + hcLbEventDelivery.acceptResolvedAddresses(result1); // It's delivered to origLb, but nothing else happens - inOrder.verify(origLb).handleResolvedAddresses(result1); + inOrder.verify(origLb).acceptResolvedAddresses(result1); assertThat(healthImpl.calls).isEmpty(); verifyNoMoreInteractions(origLb); @@ -955,9 +956,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result2); + hcLbEventDelivery.acceptResolvedAddresses(result2); - inOrder.verify(origLb).handleResolvedAddresses(result2); + inOrder.verify(origLb).acceptResolvedAddresses(result2); // Underlying subchannel is now ready deliverSubchannelState(0, ConnectivityStateInfo.forNonError(READY)); @@ -1000,9 +1001,9 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); verifyNoMoreInteractions(origLb); ServerSideCall[] serverCalls = new ServerSideCall[NUM_SUBCHANNELS]; @@ -1074,8 +1075,8 @@ public class HealthCheckingLoadBalancerFactoryTest { .setAddresses(resolvedAddressList) .setAttributes(resolutionAttrs) .build(); - hcLbEventDelivery.handleResolvedAddresses(result); - verify(origLb).handleResolvedAddresses(result); + hcLbEventDelivery.acceptResolvedAddresses(result); + verify(origLb).acceptResolvedAddresses(result); createSubchannel(0, Attributes.EMPTY); assertThat(healthImpls[0].calls).isEmpty(); deliverSubchannelState(0, ConnectivityStateInfo.forNonError(READY)); diff --git a/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java b/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java index 4a599ef232..00ce7a8373 100644 --- a/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java +++ b/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java @@ -79,9 +79,9 @@ final class CdsLoadBalancer2 extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { if (this.resolvedAddresses != null) { - return; + return true; } logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses); this.resolvedAddresses = resolvedAddresses; @@ -91,6 +91,7 @@ final class CdsLoadBalancer2 extends LoadBalancer { logger.log(XdsLogLevel.INFO, "Config: {0}", config); cdsLbState = new CdsLbState(config.name); cdsLbState.start(); + return true; } @Override @@ -209,7 +210,7 @@ final class CdsLoadBalancer2 extends LoadBalancer { if (childLb == null) { childLb = lbRegistry.getProvider(CLUSTER_RESOLVER_POLICY_NAME).newLoadBalancer(helper); } - childLb.handleResolvedAddresses( + childLb.acceptResolvedAddresses( resolvedAddresses.toBuilder().setLoadBalancingPolicyConfig(config).build()); } diff --git a/xds/src/main/java/io/grpc/xds/ClusterImplLoadBalancer.java b/xds/src/main/java/io/grpc/xds/ClusterImplLoadBalancer.java index e9796267a8..cb616e3cad 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterImplLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/ClusterImplLoadBalancer.java @@ -102,7 +102,7 @@ final class ClusterImplLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses); Attributes attributes = resolvedAddresses.getAttributes(); if (xdsClientPool == null) { @@ -129,7 +129,7 @@ final class ClusterImplLoadBalancer extends LoadBalancer { childLbHelper.updateDropPolicies(config.dropCategories); childLbHelper.updateMaxConcurrentRequests(config.maxConcurrentRequests); childLbHelper.updateSslContextProviderSupplier(config.tlsContext); - childLb.handleResolvedAddresses( + return childLb.acceptResolvedAddresses( resolvedAddresses.toBuilder() .setAttributes(attributes) .setLoadBalancingPolicyConfig(config.childPolicy.getConfig()) @@ -162,11 +162,6 @@ final class ClusterImplLoadBalancer extends LoadBalancer { } } - @Override - public boolean canHandleEmptyAddressListFromNameResolution() { - return true; - } - /** * A decorated {@link LoadBalancer.Helper} that applies configurations for connections * or requests to endpoints in the cluster. diff --git a/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java b/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java index 9b3fc83bad..e8e07c6dab 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/ClusterManagerLoadBalancer.java @@ -70,16 +70,16 @@ class ClusterManagerLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { try { resolvingAddresses = true; - handleResolvedAddressesInternal(resolvedAddresses); + return handleResolvedAddressesInternal(resolvedAddresses); } finally { resolvingAddresses = false; } } - public void handleResolvedAddressesInternal(ResolvedAddresses resolvedAddresses) { + public boolean handleResolvedAddressesInternal(ResolvedAddresses resolvedAddresses) { logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses); ClusterManagerConfig config = (ClusterManagerConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); @@ -99,7 +99,7 @@ class ClusterManagerLoadBalancer extends LoadBalancer { LoadBalancer childLb = childLbStates.get(name).lb; ResolvedAddresses childAddresses = resolvedAddresses.toBuilder().setLoadBalancingPolicyConfig(childConfig).build(); - childLb.handleResolvedAddresses(childAddresses); + childLb.acceptResolvedAddresses(childAddresses); } for (String name : childLbStates.keySet()) { if (!newChildPolicies.containsKey(name)) { @@ -109,6 +109,8 @@ class ClusterManagerLoadBalancer extends LoadBalancer { // Must update channel picker before return so that new RPCs will not be routed to deleted // clusters and resolver can remove them in service config. updateOverallBalancingState(); + + return true; } @Override @@ -126,11 +128,6 @@ class ClusterManagerLoadBalancer extends LoadBalancer { } } - @Override - public boolean canHandleEmptyAddressListFromNameResolution() { - return true; - } - @Override public void shutdown() { logger.log(XdsLogLevel.INFO, "Shutdown"); diff --git a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java index 108f406ade..bb66ff3365 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java @@ -109,7 +109,7 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses); if (xdsClientPool == null) { xdsClientPool = resolvedAddresses.getAttributes().get(InternalXdsAttributes.XDS_CLIENT_POOL); @@ -121,8 +121,10 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { logger.log(XdsLogLevel.DEBUG, "Config: {0}", config); delegate.switchTo(new ClusterResolverLbStateFactory()); this.config = config; - delegate.handleResolvedAddresses(resolvedAddresses); + return delegate.acceptResolvedAddresses(resolvedAddresses); } + + return true; } @Override @@ -166,7 +168,7 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { this.resolvedAddresses = resolvedAddresses; ClusterResolverConfig config = (ClusterResolverConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); @@ -184,6 +186,8 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { clusterStates.put(instance.cluster, state); state.start(); } + + return true; } @Override @@ -249,7 +253,7 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { if (childLb == null) { childLb = lbRegistry.getProvider(PRIORITY_POLICY_NAME).newLoadBalancer(helper); } - childLb.handleResolvedAddresses( + childLb.acceptResolvedAddresses( resolvedAddresses.toBuilder() .setLoadBalancingPolicyConfig(childConfig) .setAddresses(Collections.unmodifiableList(addresses)) diff --git a/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java b/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java index 584ac2dd16..f51959f9b6 100644 --- a/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/LeastRequestLoadBalancer.java @@ -88,7 +88,7 @@ final class LeastRequestLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { LeastRequestConfig config = (LeastRequestConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); // Config may be null if least_request is used outside xDS @@ -146,6 +146,8 @@ final class LeastRequestLoadBalancer extends LoadBalancer { for (Subchannel removedSubchannel : removedSubchannels) { shutdownSubchannel(removedSubchannel); } + + return true; } @Override diff --git a/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java b/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java index 63855a28dd..d17fab3ac6 100644 --- a/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java @@ -81,7 +81,7 @@ final class PriorityLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses); this.resolvedAddresses = resolvedAddresses; PriorityLbConfig config = (PriorityLbConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); @@ -102,6 +102,8 @@ final class PriorityLoadBalancer extends LoadBalancer { // Not to report connecting in case a pending priority bumps up on top of the current READY // priority. tryNextPriority(false); + + return true; } @Override @@ -277,7 +279,7 @@ final class PriorityLoadBalancer extends LoadBalancer { policy = newPolicy; lb.switchTo(lbProvider); } - lb.handleResolvedAddresses( + lb.acceptResolvedAddresses( resolvedAddresses.toBuilder() .setAddresses(AddressFilter.filter(resolvedAddresses.getAddresses(), priority)) .setLoadBalancingPolicyConfig(childPolicySelection.getConfig()) diff --git a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java index f0183e2c29..c72cc2a968 100644 --- a/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/RingHashLoadBalancer.java @@ -81,13 +81,13 @@ final class RingHashLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses); List addrList = resolvedAddresses.getAddresses(); if (addrList.isEmpty()) { handleNameResolutionError(Status.UNAVAILABLE.withDescription("Ring hash lb error: EDS " + "resolution was successful, but returned server addresses are empty.")); - return; + return false; } Map latestAddrs = stripAttrs(addrList); Set removedAddrs = @@ -162,6 +162,8 @@ final class RingHashLoadBalancer extends LoadBalancer { for (Subchannel subchann : removedSubchannels) { shutdownSubchannel(subchann); } + + return true; } private static List buildRing( diff --git a/xds/src/main/java/io/grpc/xds/WeightedTargetLoadBalancer.java b/xds/src/main/java/io/grpc/xds/WeightedTargetLoadBalancer.java index 5edc7bbf20..b25e542d9e 100644 --- a/xds/src/main/java/io/grpc/xds/WeightedTargetLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/WeightedTargetLoadBalancer.java @@ -61,16 +61,16 @@ final class WeightedTargetLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { try { resolvingAddresses = true; - handleResolvedAddressesInternal(resolvedAddresses); + return handleResolvedAddressesInternal(resolvedAddresses); } finally { resolvingAddresses = false; } } - public void handleResolvedAddressesInternal(ResolvedAddresses resolvedAddresses) { + public boolean handleResolvedAddressesInternal(ResolvedAddresses resolvedAddresses) { logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses); Object lbConfig = resolvedAddresses.getLoadBalancingPolicyConfig(); checkNotNull(lbConfig, "missing weighted_target lb config"); @@ -92,7 +92,7 @@ final class WeightedTargetLoadBalancer extends LoadBalancer { } targets = newTargets; for (String targetName : targets.keySet()) { - childBalancers.get(targetName).handleResolvedAddresses( + childBalancers.get(targetName).acceptResolvedAddresses( resolvedAddresses.toBuilder() .setAddresses(AddressFilter.filter(resolvedAddresses.getAddresses(), targetName)) .setLoadBalancingPolicyConfig(targets.get(targetName).policySelection.getConfig()) @@ -109,6 +109,8 @@ final class WeightedTargetLoadBalancer extends LoadBalancer { childBalancers.keySet().retainAll(targets.keySet()); childHelpers.keySet().retainAll(targets.keySet()); updateOverallBalancingState(); + + return true; } @Override @@ -122,11 +124,6 @@ final class WeightedTargetLoadBalancer extends LoadBalancer { } } - @Override - public boolean canHandleEmptyAddressListFromNameResolution() { - return true; - } - @Override public void shutdown() { logger.log(XdsLogLevel.INFO, "Shutdown"); diff --git a/xds/src/main/java/io/grpc/xds/WrrLocalityLoadBalancer.java b/xds/src/main/java/io/grpc/xds/WrrLocalityLoadBalancer.java index f320bc340e..fe996e8da1 100644 --- a/xds/src/main/java/io/grpc/xds/WrrLocalityLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/WrrLocalityLoadBalancer.java @@ -61,7 +61,7 @@ final class WrrLocalityLoadBalancer extends LoadBalancer { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses); // The configuration with the child policy is combined with the locality weights @@ -76,7 +76,7 @@ final class WrrLocalityLoadBalancer extends LoadBalancer { Status unavailable = Status.UNAVAILABLE.withDescription("wrr_locality error: no locality weights provided"); helper.updateBalancingState(TRANSIENT_FAILURE, new ErrorPicker(unavailable)); - return; + return false; } // Weighted target LB expects a WeightedPolicySelection for each locality as it will create a @@ -97,7 +97,7 @@ final class WrrLocalityLoadBalancer extends LoadBalancer { .discard(InternalXdsAttributes.ATTR_LOCALITY_WEIGHTS).build()).build(); switchLb.switchTo(lbRegistry.getProvider(WEIGHTED_TARGET_POLICY_NAME)); - switchLb.handleResolvedAddresses( + return switchLb.acceptResolvedAddresses( resolvedAddresses.toBuilder() .setLoadBalancingPolicyConfig(new WeightedTargetConfig(weightedPolicySelections)) .build()); diff --git a/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java b/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java index a90df303e0..8577df8792 100644 --- a/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java +++ b/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java @@ -130,7 +130,7 @@ public class CdsLoadBalancer2Test { lbRegistry.register(new FakeLoadBalancerProvider("least_request_experimental", new LeastRequestLoadBalancerProvider())); loadBalancer = new CdsLoadBalancer2(helper, lbRegistry); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(Collections.emptyList()) .setAttributes( @@ -627,8 +627,9 @@ public class CdsLoadBalancer2Test { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { config = resolvedAddresses.getLoadBalancingPolicyConfig(); + return true; } @Override diff --git a/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java index 582747aecb..96c711713e 100644 --- a/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/ClusterImplLoadBalancerTest.java @@ -282,7 +282,7 @@ public class ClusterImplLoadBalancerTest { config = new ClusterImplConfig(CLUSTER, EDS_SERVICE_NAME, LRS_SERVER_INFO, null, Collections.singletonList(DropOverload.create("lb", 1_000_000)), new PolicySelection(weightedTargetProvider, weightedTargetConfig), null); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(Collections.singletonList(endpoint)) .setAttributes( @@ -571,7 +571,7 @@ public class ClusterImplLoadBalancerTest { private void deliverAddressesAndConfig(List addresses, ClusterImplConfig config) { - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(addresses) .setAttributes( @@ -677,10 +677,11 @@ public class ClusterImplLoadBalancerTest { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { addresses = resolvedAddresses.getAddresses(); config = resolvedAddresses.getLoadBalancingPolicyConfig(); attributes = resolvedAddresses.getAttributes(); + return true; } @Override diff --git a/xds/src/test/java/io/grpc/xds/ClusterManagerLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterManagerLoadBalancerTest.java index c83c9c4060..7095865986 100644 --- a/xds/src/test/java/io/grpc/xds/ClusterManagerLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/ClusterManagerLoadBalancerTest.java @@ -267,7 +267,7 @@ public class ClusterManagerLoadBalancerTest { } private void deliverResolvedAddresses(final Map childPolicies, boolean failing) { - clusterManagerLoadBalancer.handleResolvedAddresses( + clusterManagerLoadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(Collections.emptyList()) .setLoadBalancingPolicyConfig(buildConfig(childPolicies, failing)) @@ -348,12 +348,14 @@ public class ClusterManagerLoadBalancerTest { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { config = resolvedAddresses.getLoadBalancingPolicyConfig(); if (failing) { helper.updateBalancingState(TRANSIENT_FAILURE, new ErrorPicker(Status.INTERNAL)); } + + return !failing; } @Override diff --git a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java index 087b64b55e..7abfe3e27a 100644 --- a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java @@ -985,7 +985,7 @@ public class ClusterResolverLoadBalancerTest { } private void deliverLbConfig(ClusterResolverConfig config) { - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(Collections.emptyList()) .setAttributes( @@ -1221,10 +1221,11 @@ public class ClusterResolverLoadBalancerTest { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { addresses = resolvedAddresses.getAddresses(); config = resolvedAddresses.getLoadBalancingPolicyConfig(); attributes = resolvedAddresses.getAttributes(); + return true; } @Override diff --git a/xds/src/test/java/io/grpc/xds/LeastRequestLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/LeastRequestLoadBalancerTest.java index f3d9acda23..3e9c522203 100644 --- a/xds/src/test/java/io/grpc/xds/LeastRequestLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/LeastRequestLoadBalancerTest.java @@ -155,7 +155,7 @@ public class LeastRequestLoadBalancerTest { @Test public void pickAfterResolved() throws Exception { final Subchannel readySubchannel = subchannels.values().iterator().next(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build()); deliverSubchannelState(readySubchannel, ConnectivityStateInfo.forNonError(READY)); @@ -206,7 +206,7 @@ public class LeastRequestLoadBalancerTest { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(currentServers).setAttributes(affinity) .build()); @@ -228,7 +228,7 @@ public class LeastRequestLoadBalancerTest { // This time with Attributes List latestServers = Lists.newArrayList(oldEag2, newEag); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(latestServers).setAttributes(affinity).build()); verify(newSubchannel, times(1)).requestConnection(); @@ -248,7 +248,7 @@ public class LeastRequestLoadBalancerTest { assertThat(getList(picker)).containsExactly(oldSubchannel, newSubchannel); // test going from non-empty to empty - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(Collections.emptyList()) .setAttributes(affinity) @@ -263,7 +263,7 @@ public class LeastRequestLoadBalancerTest { @Test public void pickAfterStateChange() throws Exception { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .build()); Subchannel subchannel = loadBalancer.getSubchannels().iterator().next(); @@ -305,7 +305,7 @@ public class LeastRequestLoadBalancerTest { final LeastRequestConfig oldConfig = new LeastRequestConfig(4); final LeastRequestConfig newConfig = new LeastRequestConfig(6); final Subchannel readySubchannel = subchannels.values().iterator().next(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity) .setLoadBalancingPolicyConfig(oldConfig).build()); deliverSubchannelState(readySubchannel, ConnectivityStateInfo.forNonError(READY)); @@ -317,7 +317,7 @@ public class LeastRequestLoadBalancerTest { pickerCaptor.getValue().pickSubchannel(mockArgs); verify(mockRandom, times(oldConfig.choiceCount)).nextInt(1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity) .setLoadBalancingPolicyConfig(newConfig).build()); verify(mockHelper, times(3)) @@ -332,7 +332,7 @@ public class LeastRequestLoadBalancerTest { @Test public void ignoreShutdownSubchannelStateChange() { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .build()); inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class)); @@ -351,7 +351,7 @@ public class LeastRequestLoadBalancerTest { @Test public void stayTransientFailureUntilReady() { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .build()); @@ -389,7 +389,7 @@ public class LeastRequestLoadBalancerTest { @Test public void refreshNameResolutionWhenSubchannelConnectionBroken() { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .build()); @@ -419,7 +419,7 @@ public class LeastRequestLoadBalancerTest { public void pickerLeastRequest() throws Exception { int choiceCount = 2; // This should add inFlight counters to all subchannels. - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .setLoadBalancingPolicyConfig(new LeastRequestConfig(choiceCount)) .build()); @@ -505,7 +505,7 @@ public class LeastRequestLoadBalancerTest { public void nameResolutionErrorWithActiveChannels() throws Exception { int choiceCount = 8; final Subchannel readySubchannel = subchannels.values().iterator().next(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setLoadBalancingPolicyConfig(new LeastRequestConfig(choiceCount)) .setAddresses(servers).setAttributes(affinity).build()); @@ -538,7 +538,7 @@ public class LeastRequestLoadBalancerTest { Subchannel sc2 = subchannelIterator.next(); Subchannel sc3 = subchannelIterator.next(); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) .build()); verify(sc1, times(1)).requestConnection(); diff --git a/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java b/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java index ecc0112a2e..8286e198cc 100644 --- a/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java +++ b/xds/src/test/java/io/grpc/xds/MetadataLoadBalancerProvider.java @@ -108,11 +108,13 @@ public class MetadataLoadBalancerProvider extends LoadBalancerProvider { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { MetadataLoadBalancerConfig config = (MetadataLoadBalancerConfig) resolvedAddresses.getLoadBalancingPolicyConfig(); helper.setMetadata(config.metadataKey, config.metadataValue); - delegateLb.handleResolvedAddresses(resolvedAddresses); + delegateLb.acceptResolvedAddresses(resolvedAddresses); + + return true; } } diff --git a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java index 47888a7980..8e46922a95 100644 --- a/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/PriorityLoadBalancerTest.java @@ -157,7 +157,7 @@ public class PriorityLoadBalancerTest { ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1, "p2", priorityChildConfig2), ImmutableList.of("p0", "p1", "p2")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(addresses) .setAttributes(attributes) @@ -166,7 +166,7 @@ public class PriorityLoadBalancerTest { assertThat(fooBalancers).hasSize(1); assertThat(barBalancers).isEmpty(); LoadBalancer fooBalancer0 = Iterables.getOnlyElement(fooBalancers); - verify(fooBalancer0).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(fooBalancer0).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); ResolvedAddresses addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(addressesReceived.getAddresses()).isEmpty(); assertThat(addressesReceived.getAttributes()).isEqualTo(attributes); @@ -177,7 +177,7 @@ public class PriorityLoadBalancerTest { assertThat(fooBalancers).hasSize(1); assertThat(barBalancers).hasSize(1); LoadBalancer barBalancer0 = Iterables.getOnlyElement(barBalancers); - verify(barBalancer0).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(barBalancer0).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(Iterables.getOnlyElement(addressesReceived.getAddresses()).getAddresses()) .containsExactly(socketAddress); @@ -189,7 +189,7 @@ public class PriorityLoadBalancerTest { assertThat(fooBalancers).hasSize(2); assertThat(barBalancers).hasSize(1); LoadBalancer fooBalancer1 = Iterables.getLast(fooBalancers); - verify(fooBalancer1).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(fooBalancer1).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(addressesReceived.getAddresses()).isEmpty(); assertThat(addressesReceived.getAttributes()).isEqualTo(attributes); @@ -206,14 +206,14 @@ public class PriorityLoadBalancerTest { ImmutableMap.of("p1", new PriorityChildConfig(new PolicySelection(barLbProvider, newBarConfig), true)), ImmutableList.of("p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(newAddresses) .setLoadBalancingPolicyConfig(newPriorityLbConfig) .build()); assertThat(fooBalancers).hasSize(2); assertThat(barBalancers).hasSize(1); - verify(barBalancer0, times(2)).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(barBalancer0, times(2)).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); addressesReceived = resolvedAddressesCaptor.getValue(); assertThat(Iterables.getOnlyElement(addressesReceived.getAddresses()).getAddresses()) .containsExactly(newSocketAddress); @@ -238,7 +238,7 @@ public class PriorityLoadBalancerTest { PriorityLbConfig priorityLbConfig = new PriorityLbConfig(ImmutableMap.of("p0", priorityChildConfig0), ImmutableList.of("p0")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -250,7 +250,7 @@ public class PriorityLoadBalancerTest { priorityLbConfig = new PriorityLbConfig(ImmutableMap.of("p1", priorityChildConfig1), ImmutableList.of("p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -281,7 +281,7 @@ public class PriorityLoadBalancerTest { ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1, "p2", priorityChildConfig2, "p3", priorityChildConfig3), ImmutableList.of("p0", "p1", "p2", "p3")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -409,7 +409,7 @@ public class PriorityLoadBalancerTest { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -445,7 +445,7 @@ public class PriorityLoadBalancerTest { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -483,7 +483,7 @@ public class PriorityLoadBalancerTest { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -516,7 +516,7 @@ public class PriorityLoadBalancerTest { // resolution update without priority change does not trigger failover Attributes.Key fooKey = Attributes.Key.create("fooKey"); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -545,7 +545,7 @@ public class PriorityLoadBalancerTest { ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1, "p2", priorityChildConfig2, "p3", priorityChildConfig3), ImmutableList.of("p0", "p1", "p2", "p3")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -643,7 +643,7 @@ public class PriorityLoadBalancerTest { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) @@ -670,7 +670,7 @@ public class PriorityLoadBalancerTest { new PriorityLbConfig( ImmutableMap.of("p0", priorityChildConfig0, "p1", priorityChildConfig1), ImmutableList.of("p0", "p1")); - priorityLb.handleResolvedAddresses( + priorityLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(priorityLbConfig) diff --git a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java index 8b205e8ad8..5ade3cff2d 100644 --- a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java @@ -156,7 +156,7 @@ public class RingHashLoadBalancerTest { public void subchannelLazyConnectUntilPicked() { RingHashConfig config = new RingHashConfig(10, 100); List servers = createWeightedServerAddrs(1); // one server - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper).createSubchannel(any(CreateSubchannelArgs.class)); @@ -187,7 +187,7 @@ public class RingHashLoadBalancerTest { public void subchannelNotAutoReconnectAfterReenteringIdle() { RingHashConfig config = new RingHashConfig(10, 100); List servers = createWeightedServerAddrs(1); // one server - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); Subchannel subchannel = Iterables.getOnlyElement(subchannels.values()); @@ -217,7 +217,7 @@ public class RingHashLoadBalancerTest { RingHashConfig config = new RingHashConfig(10, 100); List servers = createWeightedServerAddrs(1, 1); InOrder inOrder = Mockito.inOrder(helper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); inOrder.verify(helper, times(2)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -278,7 +278,7 @@ public class RingHashLoadBalancerTest { RingHashConfig config = new RingHashConfig(10, 100); List servers = createWeightedServerAddrs(1, 1, 1, 1); InOrder inOrder = Mockito.inOrder(helper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); inOrder.verify(helper, times(4)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -336,7 +336,7 @@ public class RingHashLoadBalancerTest { public void subchannelStayInTransientFailureUntilBecomeReady() { RingHashConfig config = new RingHashConfig(10, 100); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -378,7 +378,7 @@ public class RingHashLoadBalancerTest { RingHashConfig config = new RingHashConfig(10, 100); List servers = createWeightedServerAddrs(1, 1, 1); InOrder inOrder = Mockito.inOrder(helper); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -394,7 +394,7 @@ public class RingHashLoadBalancerTest { verifyConnection(1); servers = createWeightedServerAddrs(1,1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); inOrder.verify(helper) @@ -422,7 +422,7 @@ public class RingHashLoadBalancerTest { public void ignoreShutdownSubchannelStateChange() { RingHashConfig config = new RingHashConfig(10, 100); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -442,7 +442,7 @@ public class RingHashLoadBalancerTest { public void deterministicPickWithHostsPartiallyRemoved() { RingHashConfig config = new RingHashConfig(10, 100); List servers = createWeightedServerAddrs(1, 1, 1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); InOrder inOrder = Mockito.inOrder(helper); @@ -470,7 +470,7 @@ public class RingHashLoadBalancerTest { Attributes attr = addr.getAttributes().toBuilder().set(CUSTOM_KEY, "custom value").build(); updatedServers.add(new EquivalentAddressGroup(addr.getAddresses(), attr)); } - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(updatedServers).setLoadBalancingPolicyConfig(config).build()); verify(subchannels.get(Collections.singletonList(servers.get(0)))) @@ -487,7 +487,7 @@ public class RingHashLoadBalancerTest { public void deterministicPickWithNewHostsAdded() { RingHashConfig config = new RingHashConfig(10, 100); List servers = createWeightedServerAddrs(1, 1); // server0 and server1 - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); InOrder inOrder = Mockito.inOrder(helper); @@ -511,7 +511,7 @@ public class RingHashLoadBalancerTest { assertThat(subchannel.getAddresses()).isEqualTo(servers.get(1)); servers = createWeightedServerAddrs(1, 1, 1, 1, 1); // server2, server3, server4 added - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); inOrder.verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -526,7 +526,7 @@ public class RingHashLoadBalancerTest { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -583,7 +583,7 @@ public class RingHashLoadBalancerTest { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -649,7 +649,7 @@ public class RingHashLoadBalancerTest { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -687,7 +687,7 @@ public class RingHashLoadBalancerTest { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -718,7 +718,7 @@ public class RingHashLoadBalancerTest { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -749,7 +749,7 @@ public class RingHashLoadBalancerTest { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -784,7 +784,7 @@ public class RingHashLoadBalancerTest { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -822,7 +822,7 @@ public class RingHashLoadBalancerTest { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -864,7 +864,7 @@ public class RingHashLoadBalancerTest { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -908,7 +908,7 @@ public class RingHashLoadBalancerTest { // Map each server address to exactly one ring entry. RingHashConfig config = new RingHashConfig(3, 3); List servers = createWeightedServerAddrs(1, 1, 1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -943,7 +943,7 @@ public class RingHashLoadBalancerTest { public void hostSelectionProportionalToWeights() { RingHashConfig config = new RingHashConfig(10000, 100000); // large ring List servers = createWeightedServerAddrs(1, 10, 100); // 1:10:100 - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -979,7 +979,7 @@ public class RingHashLoadBalancerTest { public void hostSelectionProportionalToRepeatedAddressCount() { RingHashConfig config = new RingHashConfig(10000, 100000); List servers = createRepeatedServerAddrs(1, 10, 100); // 1:10:100 - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); @@ -1027,7 +1027,7 @@ public class RingHashLoadBalancerTest { public void nameResolutionErrorWithActiveSubchannels() { RingHashConfig config = new RingHashConfig(10, 100); List servers = createWeightedServerAddrs(1); - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(servers).setLoadBalancingPolicyConfig(config).build()); verify(helper).createSubchannel(any(CreateSubchannelArgs.class)); diff --git a/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java index 9bd33f0f18..41d2c75e17 100644 --- a/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/WeightedTargetLoadBalancerTest.java @@ -200,7 +200,7 @@ public class WeightedTargetLoadBalancerTest { eag2 = AddressFilter.setPathFilter(eag2, ImmutableList.of("target2")); EquivalentAddressGroup eag3 = new EquivalentAddressGroup(socketAddresses[3]); eag3 = AddressFilter.setPathFilter(eag3, ImmutableList.of("target3")); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of(eag0, eag1, eag2, eag3)) .setAttributes(Attributes.newBuilder().set(fakeKey, fakeValue).build()) @@ -213,7 +213,7 @@ public class WeightedTargetLoadBalancerTest { assertThat(barLbCreated).isEqualTo(2); for (int i = 0; i < childBalancers.size(); i++) { - verify(childBalancers.get(i)).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(childBalancers.get(i)).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); ResolvedAddresses resolvedAddresses = resolvedAddressesCaptor.getValue(); assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo(configs[i]); assertThat(resolvedAddresses.getAttributes().get(fakeKey)).isEqualTo(fakeValue); @@ -238,7 +238,7 @@ public class WeightedTargetLoadBalancerTest { "target4", new WeightedPolicySelection( newWeights[3], new PolicySelection(fooLbProvider, newConfigs[3]))); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(newTargets)) @@ -252,7 +252,7 @@ public class WeightedTargetLoadBalancerTest { verify(childBalancers.get(0)).shutdown(); for (int i = 1; i < childBalancers.size(); i++) { verify(childBalancers.get(i), atLeastOnce()) - .handleResolvedAddresses(resolvedAddressesCaptor.capture()); + .acceptResolvedAddresses(resolvedAddressesCaptor.capture()); assertThat(resolvedAddressesCaptor.getValue().getLoadBalancingPolicyConfig()) .isEqualTo(newConfigs[i - 1]); } @@ -280,7 +280,7 @@ public class WeightedTargetLoadBalancerTest { "target2", weightedLbConfig2, // {foo, 40, config3} "target3", weightedLbConfig3); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -307,7 +307,7 @@ public class WeightedTargetLoadBalancerTest { "target2", weightedLbConfig2, // {foo, 40, config3} "target3", weightedLbConfig3); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -388,7 +388,7 @@ public class WeightedTargetLoadBalancerTest { Map targets = ImmutableMap.of( "target0", weightedLbConfig0, "target1", weightedLbConfig1); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -414,7 +414,7 @@ public class WeightedTargetLoadBalancerTest { weights[0], new PolicySelection(fakeLbProvider, configs[0])), "target3", new WeightedPolicySelection( weights[3], new PolicySelection(fakeLbProvider, configs[3]))); - weightedTargetLb.handleResolvedAddresses( + weightedTargetLb.acceptResolvedAddresses( ResolvedAddresses.newBuilder() .setAddresses(ImmutableList.of()) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) @@ -459,8 +459,9 @@ public class WeightedTargetLoadBalancerTest { } @Override - public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { + public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { helper.updateBalancingState(TRANSIENT_FAILURE, new ErrorPicker(Status.INTERNAL)); + return true; } @Override diff --git a/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java index 29777a5284..2a9b82df18 100644 --- a/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/WrrLocalityLoadBalancerTest.java @@ -129,7 +129,7 @@ public class WrrLocalityLoadBalancerTest { // Assert that the child policy and the locality weights were correctly mapped to a // WeightedTargetConfig. - verify(mockWeightedTargetLb).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(mockWeightedTargetLb).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); Object config = resolvedAddressesCaptor.getValue().getLoadBalancingPolicyConfig(); assertThat(config).isInstanceOf(WeightedTargetConfig.class); WeightedTargetConfig wtConfig = (WeightedTargetConfig) config; @@ -190,7 +190,7 @@ public class WrrLocalityLoadBalancerTest { // Assert that the child policy and the locality weights were correctly mapped to a // WeightedTargetConfig. - verify(mockWeightedTargetLb).handleResolvedAddresses(resolvedAddressesCaptor.capture()); + verify(mockWeightedTargetLb).acceptResolvedAddresses(resolvedAddressesCaptor.capture()); assertThat(resolvedAddressesCaptor.getValue().getAttributes() .get(InternalXdsAttributes.ATTR_LOCALITY_WEIGHTS)).isNull(); } @@ -219,7 +219,7 @@ public class WrrLocalityLoadBalancerTest { } private void deliverAddresses(WrrLocalityConfig config, Map localityWeights) { - loadBalancer.handleResolvedAddresses( + loadBalancer.acceptResolvedAddresses( ResolvedAddresses.newBuilder().setAddresses(ImmutableList.of(eag)).setAttributes( Attributes.newBuilder() .set(InternalXdsAttributes.ATTR_LOCALITY_WEIGHTS, localityWeights).build())