diff --git a/core/src/main/java/io/grpc/EquivalentAddressGroup.java b/core/src/main/java/io/grpc/EquivalentAddressGroup.java index d398bbef37..eac783c7e8 100644 --- a/core/src/main/java/io/grpc/EquivalentAddressGroup.java +++ b/core/src/main/java/io/grpc/EquivalentAddressGroup.java @@ -48,6 +48,7 @@ import java.util.List; public final class EquivalentAddressGroup { private final List addrs; + private final Attributes attrs; /** * {@link SocketAddress} docs say that the addresses are immutable, so we cache the hashCode. @@ -55,20 +56,36 @@ public final class EquivalentAddressGroup { private final int hashCode; /** - * List constructor. + * List constructor without {@link Attributes}. */ public EquivalentAddressGroup(List addrs) { + this(addrs, Attributes.EMPTY); + } + + /** + * List constructor with {@link Attributes}. + */ + public EquivalentAddressGroup(List addrs, Attributes attrs) { Preconditions.checkArgument(!addrs.isEmpty(), "addrs is empty"); this.addrs = Collections.unmodifiableList(new ArrayList(addrs)); + this.attrs = Preconditions.checkNotNull(attrs, "attrs"); + // Attributes may contain mutable objects, which means Attributes' hashCode may change over + // time, thus we don't cache Attributes' hashCode. hashCode = this.addrs.hashCode(); } /** - * Singleton constructor. + * Singleton constructor without Attributes. */ public EquivalentAddressGroup(SocketAddress addr) { - this.addrs = Collections.singletonList(addr); - hashCode = addrs.hashCode(); + this(addr, Attributes.EMPTY); + } + + /** + * Singleton constructor with Attributes. + */ + public EquivalentAddressGroup(SocketAddress addr, Attributes attrs) { + this(Collections.singletonList(addr), attrs); } /** @@ -78,9 +95,16 @@ public final class EquivalentAddressGroup { return addrs; } + /** + * Returns the attributes. + */ + public Attributes getAttributes() { + return attrs; + } + @Override public String toString() { - return addrs.toString(); + return "[addrs=" + addrs + ", attrs=" + attrs + "]"; } @Override @@ -89,6 +113,14 @@ public final class EquivalentAddressGroup { return hashCode; } + /** + * Returns true if the given object is also an {@link EquivalentAddressGroup} with an equal + * address list and equal attribute values. + * + *

Note that if the attributes include mutable values, it is possible for two objects to be + * considered equal at one point in time and not equal at another (due to concurrent mutation of + * attribute values). + */ @Override public boolean equals(Object other) { if (!(other instanceof EquivalentAddressGroup)) { @@ -104,6 +136,9 @@ public final class EquivalentAddressGroup { return false; } } + if (!attrs.equals(that.attrs)) { + return false; + } return true; } } diff --git a/core/src/main/java/io/grpc/LoadBalancer.java b/core/src/main/java/io/grpc/LoadBalancer.java index 27cad55fc4..21f8404e9c 100644 --- a/core/src/main/java/io/grpc/LoadBalancer.java +++ b/core/src/main/java/io/grpc/LoadBalancer.java @@ -34,6 +34,8 @@ package io.grpc; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.base.Preconditions; +import java.net.SocketAddress; +import java.util.ArrayList; import java.util.List; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @@ -117,11 +119,44 @@ public abstract class LoadBalancer { * *

Implementations should not modify the given {@code servers}. * + * @deprecated Implement {@link #handleResolvedAddressGroups} instead. As it is deprecated, the + * {@link ResolvedServerInfo}s from the passed-in {@link ResolvedServerInfoGroup}s + * lose all their attributes. + * * @param servers the resolved server addresses, never empty. * @param attributes extra metadata from naming system. */ - public abstract void handleResolvedAddresses( - List servers, Attributes attributes); + @Deprecated + public void handleResolvedAddresses( + List servers, Attributes attributes) { + throw new UnsupportedOperationException("This is deprecated and should not be called"); + } + + /** + * 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. + * + *

Implementations should not modify the given {@code servers}. + * + * @param servers the resolved server addresses, never empty. + * @param attributes extra metadata from naming system. + */ + @SuppressWarnings("deprecation") + public void handleResolvedAddressGroups( + List servers, Attributes attributes) { + ArrayList serverInfoGroups = + new ArrayList(servers.size()); + for (EquivalentAddressGroup eag : servers) { + ResolvedServerInfoGroup.Builder serverInfoGroupBuilder = + ResolvedServerInfoGroup.builder(eag.getAttributes()); + for (SocketAddress addr : eag.getAddresses()) { + serverInfoGroupBuilder.add(new ResolvedServerInfo(addr)); + } + serverInfoGroups.add(serverInfoGroupBuilder.build()); + } + handleResolvedAddresses(serverInfoGroups, attributes); + } /** * Handles an error from the name resolution system. diff --git a/core/src/main/java/io/grpc/NameResolver.java b/core/src/main/java/io/grpc/NameResolver.java index 1217fbd6d8..0aa73ca82e 100644 --- a/core/src/main/java/io/grpc/NameResolver.java +++ b/core/src/main/java/io/grpc/NameResolver.java @@ -123,12 +123,24 @@ public abstract class NameResolver { * *

Implementations will not modify the given {@code servers}. * + * @deprecated call {@link #onAddresses} instead * @param servers the resolved server groups, containing {@link ResolvedServerInfo} objects. An * empty list will trigger {@link #onError} * @param attributes extra metadata from naming system */ + @Deprecated void onUpdate(List servers, Attributes attributes); + /** + * Handles updates on resolved addresses and attributes. + * + *

Implementations will not modify the given {@code servers}. + * + * @param servers the resolved server addresses. An empty list will trigger {@link #onError} + * @param attributes extra metadata from naming system + */ + void onAddresses(List servers, Attributes attributes); + /** * Handles an error from the resolver. * diff --git a/core/src/main/java/io/grpc/PickFirstBalancerFactory.java b/core/src/main/java/io/grpc/PickFirstBalancerFactory.java index 7e66560028..e5e1d43170 100644 --- a/core/src/main/java/io/grpc/PickFirstBalancerFactory.java +++ b/core/src/main/java/io/grpc/PickFirstBalancerFactory.java @@ -74,13 +74,12 @@ public final class PickFirstBalancerFactory extends LoadBalancer.Factory { } @Override - public void handleResolvedAddresses( - List servers, Attributes attributes) { + public void handleResolvedAddressGroups( + List servers, Attributes attributes) { // Flatten servers list received from name resolver into single address group. This means that // as far as load balancer is concerned, there's virtually one single server with multiple // addresses so the connection will be created only for the first address (pick first). - EquivalentAddressGroup newEag = - flattenResolvedServerInfoGroupsIntoEquivalentAddressGroup(servers); + EquivalentAddressGroup newEag = flattenEquivalentAddressGroup(servers); if (subchannel == null || !newEag.equals(subchannel.getAddresses())) { if (subchannel != null) { subchannel.shutdown(); @@ -136,19 +135,18 @@ public final class PickFirstBalancerFactory extends LoadBalancer.Factory { } /** - * Flattens list of ResolvedServerInfoGroup objects into one EquivalentAddressGroup object. + * Flattens list of EquivalentAddressGroup objects into one EquivalentAddressGroup object. */ - private static EquivalentAddressGroup flattenResolvedServerInfoGroupsIntoEquivalentAddressGroup( - List groupList) { + private static EquivalentAddressGroup flattenEquivalentAddressGroup( + List groupList) { List addrs = new ArrayList(); - for (ResolvedServerInfoGroup group : groupList) { - for (ResolvedServerInfo srv : group.getResolvedServerInfoList()) { - addrs.add(srv.getAddress()); + for (EquivalentAddressGroup group : groupList) { + for (SocketAddress addr : group.getAddresses()) { + addrs.add(addr); } } return new EquivalentAddressGroup(addrs); } - } /** diff --git a/core/src/main/java/io/grpc/ResolvedServerInfo.java b/core/src/main/java/io/grpc/ResolvedServerInfo.java index 5607eab92d..46e8c6d35d 100644 --- a/core/src/main/java/io/grpc/ResolvedServerInfo.java +++ b/core/src/main/java/io/grpc/ResolvedServerInfo.java @@ -39,7 +39,10 @@ import javax.annotation.concurrent.Immutable; /** * The information about a server from a {@link NameResolver}. + * + * @deprecated This class will be removed along with {@link ResolvedServerInfoGroup}. */ +@Deprecated @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1770") @Immutable public final class ResolvedServerInfo { diff --git a/core/src/main/java/io/grpc/ResolvedServerInfoGroup.java b/core/src/main/java/io/grpc/ResolvedServerInfoGroup.java index 86cec3cfbe..d2bd9144bc 100644 --- a/core/src/main/java/io/grpc/ResolvedServerInfoGroup.java +++ b/core/src/main/java/io/grpc/ResolvedServerInfoGroup.java @@ -44,7 +44,10 @@ import javax.annotation.concurrent.Immutable; /** * A group of {@link ResolvedServerInfo}s that is returned from a {@link NameResolver}. + * + * @deprecated This class will be removed. Use {@link EquivalentAddressGroup} instead. */ +@Deprecated @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1770") @Immutable public final class ResolvedServerInfoGroup { @@ -88,7 +91,7 @@ public final class ResolvedServerInfoGroup { for (ResolvedServerInfo resolvedServerInfo : resolvedServerInfoList) { addrs.add(resolvedServerInfo.getAddress()); } - return new EquivalentAddressGroup(addrs); + return new EquivalentAddressGroup(addrs, attributes); } /** diff --git a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java index a403e262e7..02c202d97f 100644 --- a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java @@ -43,14 +43,13 @@ import io.grpc.Attributes; import io.grpc.ClientInterceptor; import io.grpc.CompressorRegistry; import io.grpc.DecompressorRegistry; +import io.grpc.EquivalentAddressGroup; import io.grpc.LoadBalancer; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; import io.grpc.NameResolver; import io.grpc.NameResolverProvider; import io.grpc.PickFirstBalancerFactory; -import io.grpc.ResolvedServerInfo; -import io.grpc.ResolvedServerInfoGroup; import java.net.SocketAddress; import java.net.URI; import java.net.URISyntaxException; @@ -373,8 +372,8 @@ public abstract class AbstractManagedChannelImplBuilder @Override public void start(final Listener listener) { - listener.onUpdate(Collections.singletonList( - ResolvedServerInfoGroup.builder().add(new ResolvedServerInfo(address)).build()), + listener.onAddresses( + Collections.singletonList(new EquivalentAddressGroup(address)), Attributes.EMPTY); } diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java index c2dfa41e41..23cea0333b 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java @@ -34,15 +34,15 @@ package io.grpc.internal; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import io.grpc.Attributes; +import io.grpc.EquivalentAddressGroup; import io.grpc.NameResolver; -import io.grpc.ResolvedServerInfo; -import io.grpc.ResolvedServerInfoGroup; import io.grpc.Status; import io.grpc.internal.SharedResourceHolder.Resource; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.URI; import java.net.UnknownHostException; +import java.util.ArrayList; import java.util.Collections; import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledExecutorService; @@ -54,6 +54,9 @@ import javax.annotation.concurrent.GuardedBy; /** * A DNS-based {@link NameResolver}. * + *

Each {@code A} or {@code AAAA} record emits an {@link EquivalentAddressGroup} in the list + * passed to {@link NameResolver.Listener#onUpdate} + * * @see DnsNameResolverFactory */ class DnsNameResolver extends NameResolver { @@ -141,11 +144,9 @@ class DnsNameResolver extends NameResolver { } try { if (System.getenv("GRPC_PROXY_EXP") != null) { - ResolvedServerInfoGroup servers = ResolvedServerInfoGroup.builder() - .add(new ResolvedServerInfo( - InetSocketAddress.createUnresolved(host, port), Attributes.EMPTY)) - .build(); - savedListener.onUpdate(Collections.singletonList(servers), Attributes.EMPTY); + EquivalentAddressGroup server = + new EquivalentAddressGroup(InetSocketAddress.createUnresolved(host, port)); + savedListener.onAddresses(Collections.singletonList(server), Attributes.EMPTY); return; } @@ -165,13 +166,13 @@ class DnsNameResolver extends NameResolver { savedListener.onError(Status.UNAVAILABLE.withCause(e)); return; } - ResolvedServerInfoGroup.Builder servers = ResolvedServerInfoGroup.builder(); + // Each address forms an EAG + ArrayList servers = new ArrayList(); for (int i = 0; i < inetAddrs.length; i++) { InetAddress inetAddr = inetAddrs[i]; - servers.add( - new ResolvedServerInfo(new InetSocketAddress(inetAddr, port), Attributes.EMPTY)); + servers.add(new EquivalentAddressGroup(new InetSocketAddress(inetAddr, port))); } - savedListener.onUpdate(Collections.singletonList(servers.build()), Attributes.EMPTY); + savedListener.onAddresses(servers, Attributes.EMPTY); } finally { synchronized (DnsNameResolver.this) { resolving = false; diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index d79ec8ae7d..5b83bc48d7 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -58,11 +58,11 @@ import io.grpc.LoadBalancer.SubchannelPicker; import io.grpc.ManagedChannel; import io.grpc.MethodDescriptor; import io.grpc.NameResolver; -import io.grpc.ResolvedServerInfoGroup; import io.grpc.Status; import io.grpc.internal.ClientCallImpl.ClientTransportProvider; import java.net.URI; import java.net.URISyntaxException; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -741,8 +741,20 @@ public final class ManagedChannelImpl extends ManagedChannel implements WithLogI this.helper = helperImpl; } + @Deprecated @Override - public void onUpdate(final List servers, final Attributes config) { + public void onUpdate( + final List servers, final Attributes config) { + ArrayList eags = + new ArrayList(servers.size()); + for (io.grpc.ResolvedServerInfoGroup infoGroup : servers) { + eags.add(infoGroup.toEquivalentAddressGroup()); + } + onAddresses(eags, config); + } + + @Override + public void onAddresses(final List servers, final Attributes config) { if (servers.isEmpty()) { onError(Status.UNAVAILABLE.withDescription("NameResolver returned an empty list")); return; @@ -756,7 +768,7 @@ public final class ManagedChannelImpl extends ManagedChannel implements WithLogI return; } try { - balancer.handleResolvedAddresses(servers, config); + balancer.handleResolvedAddressGroups(servers, config); } catch (Throwable e) { log.log( Level.WARNING, "[" + getLogId() + "] Unexpected exception from LoadBalancer", e); diff --git a/core/src/main/java/io/grpc/util/RoundRobinLoadBalancerFactory.java b/core/src/main/java/io/grpc/util/RoundRobinLoadBalancerFactory.java index 6f0890983e..2e8e1ebd2b 100644 --- a/core/src/main/java/io/grpc/util/RoundRobinLoadBalancerFactory.java +++ b/core/src/main/java/io/grpc/util/RoundRobinLoadBalancerFactory.java @@ -47,8 +47,6 @@ import io.grpc.LoadBalancer.PickSubchannelArgs; import io.grpc.LoadBalancer.Subchannel; import io.grpc.LoadBalancer.SubchannelPicker; import io.grpc.NameResolver; -import io.grpc.ResolvedServerInfo; -import io.grpc.ResolvedServerInfoGroup; import io.grpc.Status; import java.util.ArrayList; import java.util.Collection; @@ -101,11 +99,10 @@ public class RoundRobinLoadBalancerFactory extends LoadBalancer.Factory { } @Override - public void handleResolvedAddresses( - List servers, Attributes attributes) { + public void handleResolvedAddressGroups( + List servers, Attributes attributes) { Set currentAddrs = subchannels.keySet(); - Set latestAddrs = - resolvedServerInfoGroupToEquivalentAddressGroup(servers); + Set latestAddrs = stripAttrs(servers); Set addedAddrs = setsDifference(latestAddrs, currentAddrs); Set removedAddrs = setsDifference(currentAddrs, latestAddrs); @@ -184,15 +181,13 @@ public class RoundRobinLoadBalancerFactory extends LoadBalancer.Factory { } /** - * Converts list of {@link ResolvedServerInfoGroup} to {@link EquivalentAddressGroup} set. + * Converts list of {@link EquivalentAddressGroup} to {@link EquivalentAddressGroup} set and + * remove all attributes. */ - private static Set resolvedServerInfoGroupToEquivalentAddressGroup( - List groupList) { + private static Set stripAttrs(List groupList) { Set addrs = new HashSet(); - for (ResolvedServerInfoGroup group : groupList) { - for (ResolvedServerInfo server : group.getResolvedServerInfoList()) { - addrs.add(new EquivalentAddressGroup(server.getAddress())); - } + for (EquivalentAddressGroup group : groupList) { + addrs.add(new EquivalentAddressGroup(group.getAddresses())); } return addrs; } diff --git a/core/src/test/java/io/grpc/PickFirstLoadBalancerTest.java b/core/src/test/java/io/grpc/PickFirstLoadBalancerTest.java index 931ae91d06..e7e0784e9e 100644 --- a/core/src/test/java/io/grpc/PickFirstLoadBalancerTest.java +++ b/core/src/test/java/io/grpc/PickFirstLoadBalancerTest.java @@ -68,7 +68,7 @@ import org.mockito.MockitoAnnotations; @RunWith(JUnit4.class) public class PickFirstLoadBalancerTest { private PickFirstBalancer loadBalancer; - private List servers = Lists.newArrayList(); + private List servers = Lists.newArrayList(); private List socketAddresses = Lists.newArrayList(); private static final Attributes.Key FOO = Attributes.Key.of("foo"); @@ -92,7 +92,7 @@ public class PickFirstLoadBalancerTest { MockitoAnnotations.initMocks(this); for (int i = 0; i < 3; i++) { SocketAddress addr = new FakeSocketAddress("server" + i); - servers.add(ResolvedServerInfoGroup.builder().add(new ResolvedServerInfo(addr)).build()); + servers.add(new EquivalentAddressGroup(addr)); socketAddresses.add(addr); } @@ -111,7 +111,7 @@ public class PickFirstLoadBalancerTest { @Test public void pickAfterResolved() throws Exception { - loadBalancer.handleResolvedAddresses(servers, affinity); + loadBalancer.handleResolvedAddressGroups(servers, affinity); verify(mockHelper).createSubchannel(eagCaptor.capture(), attrsCaptor.capture()); verify(mockHelper).updatePicker(pickerCaptor.capture()); @@ -125,8 +125,8 @@ public class PickFirstLoadBalancerTest { @Test public void pickAfterResolvedAndUnchanged() throws Exception { - loadBalancer.handleResolvedAddresses(servers, affinity); - loadBalancer.handleResolvedAddresses(servers, affinity); + loadBalancer.handleResolvedAddressGroups(servers, affinity); + loadBalancer.handleResolvedAddressGroups(servers, affinity); verify(mockHelper).createSubchannel(any(EquivalentAddressGroup.class), any(Attributes.class)); @@ -139,8 +139,8 @@ public class PickFirstLoadBalancerTest { public void pickAfterResolvedAndChanged() throws Exception { SocketAddress socketAddr = new FakeSocketAddress("newserver"); List newSocketAddresses = Lists.newArrayList(socketAddr); - List newServers = Lists.newArrayList( - ResolvedServerInfoGroup.builder().add(new ResolvedServerInfo(socketAddr)).build()); + List newServers = + Lists.newArrayList(new EquivalentAddressGroup(socketAddr)); final Subchannel oldSubchannel = mock(Subchannel.class); final EquivalentAddressGroup oldEag = new EquivalentAddressGroup(socketAddresses); @@ -155,12 +155,12 @@ public class PickFirstLoadBalancerTest { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses(servers, affinity); + loadBalancer.handleResolvedAddressGroups(servers, affinity); inOrder.verify(mockHelper).createSubchannel(eagCaptor.capture(), any(Attributes.class)); inOrder.verify(mockHelper).updatePicker(pickerCaptor.capture()); assertEquals(socketAddresses, eagCaptor.getValue().getAddresses()); - loadBalancer.handleResolvedAddresses(newServers, affinity); + loadBalancer.handleResolvedAddressGroups(newServers, affinity); inOrder.verify(mockHelper).createSubchannel(eagCaptor.capture(), any(Attributes.class)); inOrder.verify(mockHelper).updatePicker(pickerCaptor.capture()); assertEquals(newSocketAddresses, eagCaptor.getValue().getAddresses()); @@ -187,7 +187,7 @@ public class PickFirstLoadBalancerTest { @Test public void pickAfterStateChangeAfterResolution() throws Exception { - loadBalancer.handleResolvedAddresses(servers, affinity); + loadBalancer.handleResolvedAddressGroups(servers, affinity); verify(mockHelper).updatePicker(pickerCaptor.capture()); Subchannel subchannel = pickerCaptor.getValue().pickSubchannel(mockArgs).getSubchannel(); reset(mockHelper); @@ -232,7 +232,7 @@ public class PickFirstLoadBalancerTest { loadBalancer.handleNameResolutionError(Status.NOT_FOUND.withDescription("nameResolutionError")); inOrder.verify(mockHelper).updatePicker(any(Picker.class)); - loadBalancer.handleResolvedAddresses(servers, affinity); + loadBalancer.handleResolvedAddressGroups(servers, affinity); inOrder.verify(mockHelper).createSubchannel(eq(new EquivalentAddressGroup(socketAddresses)), eq(Attributes.EMPTY)); inOrder.verify(mockHelper).updatePicker(pickerCaptor.capture()); diff --git a/core/src/test/java/io/grpc/ResolvedServerInfoTest.java b/core/src/test/java/io/grpc/ResolvedServerInfoTest.java deleted file mode 100644 index ed328a706f..0000000000 --- a/core/src/test/java/io/grpc/ResolvedServerInfoTest.java +++ /dev/null @@ -1,131 +0,0 @@ -/* - * Copyright 2016, Google Inc. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ - -package io.grpc; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.fail; - -import java.net.InetSocketAddress; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -@RunWith(JUnit4.class) -public class ResolvedServerInfoTest { - private static final Attributes.Key FOO = Attributes.Key.of("foo"); - private static final Attributes ATTRS = Attributes.newBuilder().set(FOO, "bar").build(); - - @Test - public void accessors() { - InetSocketAddress addr = InetSocketAddress.createUnresolved("foo", 123); - - ResolvedServerInfo server = new ResolvedServerInfo(addr, ATTRS); - assertEquals(addr, server.getAddress()); - assertEquals(ATTRS, server.getAttributes()); - - // unspecified attributes treated as empty - server = new ResolvedServerInfo(addr); - assertEquals(addr, server.getAddress()); - assertEquals(Attributes.EMPTY, server.getAttributes()); - } - - @Test public void cannotUseNullAddress() { - try { - new ResolvedServerInfo(null, ATTRS); - fail("Should not have been allowd to create info with null address"); - } catch (NullPointerException e) { - // expected - } - } - - @Test public void equals_true() { - ResolvedServerInfo server1 = new ResolvedServerInfo( - InetSocketAddress.createUnresolved("foo", 123), - Attributes.newBuilder().set(FOO, "bar").build()); - ResolvedServerInfo server2 = new ResolvedServerInfo( - InetSocketAddress.createUnresolved("foo", 123), - Attributes.newBuilder().set(FOO, "bar").build()); - - // sanity checks that they're not same instances - assertNotSame(server1.getAddress(), server2.getAddress()); - assertNotSame(server1.getAttributes(), server2.getAttributes()); - - assertEquals(server1, server2); - assertEquals(server1.hashCode(), server2.hashCode()); // hash code must be consistent - - // empty attributes - server1 = new ResolvedServerInfo(InetSocketAddress.createUnresolved("foo", 123)); - server2 = new ResolvedServerInfo( - InetSocketAddress.createUnresolved("foo", 123), Attributes.EMPTY); - assertEquals(server1, server2); - assertEquals(server1.hashCode(), server2.hashCode()); - } - - @Test public void equals_falseDifferentAddresses() { - ResolvedServerInfo server1 = new ResolvedServerInfo( - InetSocketAddress.createUnresolved("foo", 123), - Attributes.newBuilder().set(FOO, "bar").build()); - ResolvedServerInfo server2 = new ResolvedServerInfo( - InetSocketAddress.createUnresolved("foo", 456), - Attributes.newBuilder().set(FOO, "bar").build()); - - assertNotEquals(server1, server2); - // hash code could collide, but this assertion is safe because, in this example, they do not - assertNotEquals(server1.hashCode(), server2.hashCode()); - } - - @Test public void equals_falseDifferentAttributes() { - ResolvedServerInfo server1 = new ResolvedServerInfo( - InetSocketAddress.createUnresolved("foo", 123), - Attributes.newBuilder().set(FOO, "bar").build()); - ResolvedServerInfo server2 = new ResolvedServerInfo( - InetSocketAddress.createUnresolved("foo", 123), - Attributes.newBuilder().set(FOO, "baz").build()); - - assertNotEquals(server1, server2); - // hash code could collide, but these assertions are safe because, in these examples, they don't - assertNotEquals(server1.hashCode(), server2.hashCode()); - - // same values but extra key? still not equal - server2 = new ResolvedServerInfo( - InetSocketAddress.createUnresolved("foo", 456), - Attributes.newBuilder() - .set(FOO, "bar") - .set(Attributes.Key.of("fiz"), "buz") - .build()); - - assertNotEquals(server1, server2); - assertNotEquals(server1.hashCode(), server2.hashCode()); - } -} diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index bb88b1ab16..52f68322a7 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -42,8 +42,8 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import com.google.common.collect.Iterables; import io.grpc.Attributes; +import io.grpc.EquivalentAddressGroup; import io.grpc.NameResolver; -import io.grpc.ResolvedServerInfoGroup; import io.grpc.Status; import io.grpc.internal.SharedResourceHolder.Resource; import java.net.InetAddress; @@ -104,7 +104,7 @@ public class DnsNameResolverTest { @Mock private NameResolver.Listener mockListener; @Captor - private ArgumentCaptor> resultCaptor; + private ArgumentCaptor> resultCaptor; @Captor private ArgumentCaptor statusCaptor; @@ -149,16 +149,16 @@ public class DnsNameResolverTest { MockResolver resolver = new MockResolver(name, 81, answer1, answer2); resolver.start(mockListener); assertEquals(1, fakeExecutor.runDueTasks()); - verify(mockListener).onUpdate(resultCaptor.capture(), any(Attributes.class)); + verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class)); assertEquals(name, resolver.invocations.poll()); - assertAnswerMatches(answer1, 81, Iterables.getOnlyElement(resultCaptor.getValue())); + assertAnswerMatches(answer1, 81, resultCaptor.getValue()); assertEquals(0, fakeClock.numPendingTasks()); resolver.refresh(); assertEquals(1, fakeExecutor.runDueTasks()); - verify(mockListener, times(2)).onUpdate(resultCaptor.capture(), any(Attributes.class)); + verify(mockListener, times(2)).onAddresses(resultCaptor.capture(), any(Attributes.class)); assertEquals(name, resolver.invocations.poll()); - assertAnswerMatches(answer2, 81, Iterables.getOnlyElement(resultCaptor.getValue())); + assertAnswerMatches(answer2, 81, resultCaptor.getValue()); assertEquals(0, fakeClock.numPendingTasks()); resolver.shutdown(); @@ -201,9 +201,9 @@ public class DnsNameResolverTest { fakeClock.forwardNanos(1); assertEquals(0, fakeClock.numPendingTasks()); assertEquals(1, fakeExecutor.runDueTasks()); - verify(mockListener).onUpdate(resultCaptor.capture(), any(Attributes.class)); + verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class)); assertEquals(name, resolver.invocations.poll()); - assertAnswerMatches(answer, 81, Iterables.getOnlyElement(resultCaptor.getValue())); + assertAnswerMatches(answer, 81, resultCaptor.getValue()); verifyNoMoreInteractions(mockListener); } @@ -229,9 +229,9 @@ public class DnsNameResolverTest { assertEquals(1, fakeExecutor.runDueTasks()); // Refresh cancelled the retry assertEquals(0, fakeClock.numPendingTasks()); - verify(mockListener).onUpdate(resultCaptor.capture(), any(Attributes.class)); + verify(mockListener).onAddresses(resultCaptor.capture(), any(Attributes.class)); assertEquals(name, resolver.invocations.poll()); - assertAnswerMatches(answer, 81, Iterables.getOnlyElement(resultCaptor.getValue())); + assertAnswerMatches(answer, 81, resultCaptor.getValue()); verifyNoMoreInteractions(mockListener); } @@ -286,12 +286,13 @@ public class DnsNameResolverTest { return list; } - private static void assertAnswerMatches(InetAddress[] addrs, int port, - ResolvedServerInfoGroup result) { - assertEquals(addrs.length, result.getResolvedServerInfoList().size()); + private static void assertAnswerMatches( + InetAddress[] addrs, int port, List results) { + assertEquals(addrs.length, results.size()); for (int i = 0; i < addrs.length; i++) { - InetSocketAddress socketAddr = (InetSocketAddress) result.getResolvedServerInfoList().get( - i).getAddress(); + EquivalentAddressGroup addrGroup = results.get(i); + InetSocketAddress socketAddr = + (InetSocketAddress) Iterables.getOnlyElement(addrGroup.getAddresses()); assertEquals("Addr " + i, port, socketAddr.getPort()); assertEquals("Addr " + i, addrs[i], socketAddr.getAddress()); } diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java index 83935084d3..8770fc907f 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java @@ -63,8 +63,6 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.MethodType; import io.grpc.NameResolver; -import io.grpc.ResolvedServerInfo; -import io.grpc.ResolvedServerInfoGroup; import io.grpc.Status; import io.grpc.StringMarshaller; import io.grpc.internal.TestUtils.MockClientTransportInfo; @@ -108,9 +106,7 @@ public class ManagedChannelImplIdlenessTest { .setResponseMarshaller(new IntegerMarshaller()) .build(); - private final List servers = Lists.newArrayList(); - private final List addressGroupList = - new ArrayList(); + private final List servers = Lists.newArrayList(); @Mock private ObjectPool timerServicePool; @Mock private ObjectPool executorPool; @@ -147,13 +143,11 @@ public class ManagedChannelImplIdlenessTest { newTransports = TestUtils.captureTransports(mockTransportFactory); for (int i = 0; i < 2; i++) { - ResolvedServerInfoGroup.Builder resolvedServerInfoGroup = ResolvedServerInfoGroup.builder(); + ArrayList addrs = Lists.newArrayList(); for (int j = 0; j < 2; j++) { - resolvedServerInfoGroup.add( - new ResolvedServerInfo(new FakeSocketAddress("servergroup" + i + "server" + j))); + addrs.add(new FakeSocketAddress("servergroup" + i + "server" + j)); } - servers.add(resolvedServerInfoGroup.build()); - addressGroupList.add(resolvedServerInfoGroup.build().toEquivalentAddressGroup()); + servers.add(new EquivalentAddressGroup(addrs)); } verify(mockNameResolverFactory).newNameResolver(any(URI.class), any(Attributes.class)); // Verify the initial idleness @@ -179,8 +173,8 @@ public class ManagedChannelImplIdlenessTest { verify(mockNameResolver).start(nameResolverListenerCaptor.capture()); // Simulate new address resolved to make sure the LoadBalancer is correctly linked to // the NameResolver. - nameResolverListenerCaptor.getValue().onUpdate(servers, Attributes.EMPTY); - verify(mockLoadBalancer).handleResolvedAddresses(servers, Attributes.EMPTY); + nameResolverListenerCaptor.getValue().onAddresses(servers, Attributes.EMPTY); + verify(mockLoadBalancer).handleResolvedAddressGroups(servers, Attributes.EMPTY); } @Test @@ -245,7 +239,7 @@ public class ManagedChannelImplIdlenessTest { @Test public void realTransportsHoldsOffIdleness() throws Exception { - final EquivalentAddressGroup addressGroup = addressGroupList.get(1); + final EquivalentAddressGroup addressGroup = servers.get(1); // Start a call, which goes to delayed transport ClientCall call = channel.newCall(method, CallOptions.DEFAULT); @@ -313,7 +307,7 @@ public class ManagedChannelImplIdlenessTest { assertFalse(channel.inUseStateAggregator.isInUse()); // Now make an RPC on an OOB channel - ManagedChannel oob = helper.createOobChannel(addressGroupList.get(0), "oobauthority"); + ManagedChannel oob = helper.createOobChannel(servers.get(0), "oobauthority"); verify(mockTransportFactory, never()) .newClientTransport(any(SocketAddress.class), same("oobauthority"), same(USER_AGENT)); ClientCall oobCall = oob.newCall(method, CallOptions.DEFAULT); diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index d7ab3a326e..52ad8fc1d0 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -82,8 +82,6 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.MethodType; import io.grpc.NameResolver; -import io.grpc.ResolvedServerInfo; -import io.grpc.ResolvedServerInfoGroup; import io.grpc.SecurityLevel; import io.grpc.Status; import io.grpc.StringMarshaller; @@ -140,7 +138,6 @@ public class ManagedChannelImplTest { private URI expectedUri; private final SocketAddress socketAddress = new SocketAddress() {}; private final EquivalentAddressGroup addressGroup = new EquivalentAddressGroup(socketAddress); - private final ResolvedServerInfo server = new ResolvedServerInfo(socketAddress, Attributes.EMPTY); private final FakeClock timer = new FakeClock(); private final FakeClock executor = new FakeClock(); private final FakeClock oobExecutor = new FakeClock(); @@ -515,8 +512,8 @@ public class ManagedChannelImplTest { createChannel(nameResolverFactory, NO_INTERCEPTOR); verify(mockLoadBalancerFactory).newLoadBalancer(any(Helper.class)); - doThrow(ex).when(mockLoadBalancer).handleResolvedAddresses( - Matchers.>anyObject(), any(Attributes.class)); + doThrow(ex).when(mockLoadBalancer).handleResolvedAddressGroups( + Matchers.>anyObject(), any(Attributes.class)); // NameResolver returns addresses. nameResolverFactory.allResolved(); @@ -561,16 +558,10 @@ public class ManagedChannelImplTest { return "badAddress"; } }; - final ResolvedServerInfo goodServer = new ResolvedServerInfo(goodAddress, Attributes.EMPTY); - final ResolvedServerInfo badServer = new ResolvedServerInfo(badAddress, Attributes.EMPTY); InOrder inOrder = inOrder(mockLoadBalancer); - ResolvedServerInfoGroup serverInfoGroup = ResolvedServerInfoGroup.builder() - .add(badServer) - .add(goodServer) - .build(); - FakeNameResolverFactory nameResolverFactory = - new FakeNameResolverFactory(serverInfoGroup.getResolvedServerInfoList()); + List resolvedAddrs = Arrays.asList(badAddress, goodAddress); + FakeNameResolverFactory nameResolverFactory = new FakeNameResolverFactory(resolvedAddrs); createChannel(nameResolverFactory, NO_INTERCEPTOR); // Start the call @@ -580,10 +571,10 @@ public class ManagedChannelImplTest { executor.runDueTasks(); // Simulate name resolution results - inOrder.verify(mockLoadBalancer).handleResolvedAddresses( - eq(Arrays.asList(serverInfoGroup)), eq(Attributes.EMPTY)); - Subchannel subchannel = helper.createSubchannel( - serverInfoGroup.toEquivalentAddressGroup(), Attributes.EMPTY); + EquivalentAddressGroup addressGroup = new EquivalentAddressGroup(resolvedAddrs); + inOrder.verify(mockLoadBalancer).handleResolvedAddressGroups( + eq(Arrays.asList(addressGroup)), eq(Attributes.EMPTY)); + Subchannel subchannel = helper.createSubchannel(addressGroup, Attributes.EMPTY); when(mockPicker.pickSubchannel(any(PickSubchannelArgs.class))) .thenReturn(PickResult.withSubchannel(subchannel)); subchannel.requestConnection(); @@ -644,17 +635,11 @@ public class ManagedChannelImplTest { return "addr2"; } }; - final ResolvedServerInfo server1 = new ResolvedServerInfo(addr1, Attributes.EMPTY); - final ResolvedServerInfo server2 = new ResolvedServerInfo(addr2, Attributes.EMPTY); InOrder inOrder = inOrder(mockLoadBalancer); - ResolvedServerInfoGroup serverInfoGroup = ResolvedServerInfoGroup.builder() - .add(server1) - .add(server2) - .build(); + List resolvedAddrs = Arrays.asList(addr1, addr2); - FakeNameResolverFactory nameResolverFactory = - new FakeNameResolverFactory(serverInfoGroup.getResolvedServerInfoList()); + FakeNameResolverFactory nameResolverFactory = new FakeNameResolverFactory(resolvedAddrs); createChannel(nameResolverFactory, NO_INTERCEPTOR); // Start a wait-for-ready call @@ -669,10 +654,10 @@ public class ManagedChannelImplTest { executor.runDueTasks(); // Simulate name resolution results - inOrder.verify(mockLoadBalancer).handleResolvedAddresses( - eq(Arrays.asList(serverInfoGroup)), eq(Attributes.EMPTY)); - Subchannel subchannel = helper.createSubchannel( - serverInfoGroup.toEquivalentAddressGroup(), Attributes.EMPTY); + EquivalentAddressGroup addressGroup = new EquivalentAddressGroup(resolvedAddrs); + inOrder.verify(mockLoadBalancer).handleResolvedAddressGroups( + eq(Arrays.asList(addressGroup)), eq(Attributes.EMPTY)); + Subchannel subchannel = helper.createSubchannel(addressGroup, Attributes.EMPTY); when(mockPicker.pickSubchannel(any(PickSubchannelArgs.class))) .thenReturn(PickResult.withSubchannel(subchannel)); subchannel.requestConnection(); @@ -1005,8 +990,6 @@ public class ManagedChannelImplTest { */ @Test public void informationPropagatedToNewStreamAndCallCredentials() { - ResolvedServerInfoGroup serverInfoGroup = ResolvedServerInfoGroup.builder() - .add(server).build(); createChannel(new FakeNameResolverFactory(true), NO_INTERCEPTOR); CallOptions callOptions = CallOptions.DEFAULT.withCallCredentials(creds); final Context.Key testKey = Context.key("testing"); @@ -1034,8 +1017,8 @@ public class ManagedChannelImplTest { call.start(mockCallListener, new Metadata()); // Simulate name resolution results - Subchannel subchannel = helper.createSubchannel( - serverInfoGroup.toEquivalentAddressGroup(), Attributes.EMPTY); + EquivalentAddressGroup addressGroup = new EquivalentAddressGroup(socketAddress); + Subchannel subchannel = helper.createSubchannel(addressGroup, Attributes.EMPTY); subchannel.requestConnection(); verify(mockTransportFactory).newClientTransport( same(socketAddress), eq(authority), eq(userAgent)); @@ -1122,19 +1105,18 @@ public class ManagedChannelImplTest { } private class FakeNameResolverFactory extends NameResolver.Factory { - final List servers; + final List servers; final boolean resolvedAtStart; final ArrayList resolvers = new ArrayList(); FakeNameResolverFactory(boolean resolvedAtStart) { this.resolvedAtStart = resolvedAtStart; - servers = Collections.singletonList(ResolvedServerInfoGroup.builder().add(server).build()); + servers = Collections.singletonList(new EquivalentAddressGroup(socketAddress)); } - FakeNameResolverFactory(List servers) { + FakeNameResolverFactory(List servers) { resolvedAtStart = true; - this.servers = Collections.singletonList( - ResolvedServerInfoGroup.builder().addAll(servers).build()); + this.servers = Collections.singletonList(new EquivalentAddressGroup(servers)); } public FakeNameResolverFactory() { @@ -1180,7 +1162,7 @@ public class ManagedChannelImplTest { } void resolved() { - listener.onUpdate(servers, Attributes.EMPTY); + listener.onAddresses(servers, Attributes.EMPTY); } @Override public void shutdown() { diff --git a/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java b/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java index 2414ab20ac..b263d8d4b1 100644 --- a/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java +++ b/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java @@ -58,8 +58,6 @@ import io.grpc.LoadBalancer; import io.grpc.LoadBalancer.Helper; import io.grpc.LoadBalancer.PickSubchannelArgs; import io.grpc.LoadBalancer.Subchannel; -import io.grpc.ResolvedServerInfo; -import io.grpc.ResolvedServerInfoGroup; import io.grpc.Status; import io.grpc.util.RoundRobinLoadBalancerFactory.Picker; import io.grpc.util.RoundRobinLoadBalancerFactory.RoundRobinLoadBalancer; @@ -86,7 +84,7 @@ import org.mockito.stubbing.Answer; @RunWith(JUnit4.class) public class RoundRobinLoadBalancerTest { private RoundRobinLoadBalancer loadBalancer; - private Map servers = Maps.newHashMap(); + private List servers = Lists.newArrayList(); private Map subchannels = Maps.newLinkedHashMap(); private static final Attributes.Key MAJOR_KEY = Attributes.Key.of("major-key"); private Attributes affinity = Attributes.newBuilder().set(MAJOR_KEY, "I got the keys").build(); @@ -109,7 +107,7 @@ public class RoundRobinLoadBalancerTest { for (int i = 0; i < 3; i++) { SocketAddress addr = new FakeSocketAddress("server" + i); EquivalentAddressGroup eag = new EquivalentAddressGroup(addr); - servers.put(ResolvedServerInfoGroup.builder().add(new ResolvedServerInfo(addr)).build(), eag); + servers.add(eag); subchannels.put(eag, mock(Subchannel.class)); } @@ -136,7 +134,7 @@ public class RoundRobinLoadBalancerTest { @Test public void pickAfterResolved() throws Exception { final Subchannel readySubchannel = subchannels.values().iterator().next(); - loadBalancer.handleResolvedAddresses(Lists.newArrayList(servers.keySet()), affinity); + loadBalancer.handleResolvedAddressGroups(servers, affinity); loadBalancer.handleSubchannelState(readySubchannel, ConnectivityStateInfo.forNonError(READY)); verify(mockHelper, times(3)).createSubchannel(eagCaptor.capture(), @@ -176,11 +174,10 @@ public class RoundRobinLoadBalancerTest { subchannels2.put(new EquivalentAddressGroup(removedAddr), removedSubchannel); subchannels2.put(new EquivalentAddressGroup(oldAddr), oldSubchannel); - List currentServers = Lists.newArrayList( - ResolvedServerInfoGroup.builder() - .add(new ResolvedServerInfo(removedAddr)) - .add(new ResolvedServerInfo(oldAddr)) - .build()); + List currentServers = + Lists.newArrayList( + new EquivalentAddressGroup(removedAddr), + new EquivalentAddressGroup(oldAddr)); doAnswer(new Answer() { @Override @@ -190,7 +187,7 @@ public class RoundRobinLoadBalancerTest { } }).when(mockHelper).createSubchannel(any(EquivalentAddressGroup.class), any(Attributes.class)); - loadBalancer.handleResolvedAddresses(currentServers, affinity); + loadBalancer.handleResolvedAddressGroups(currentServers, affinity); InOrder inOrder = inOrder(mockHelper); @@ -209,13 +206,12 @@ public class RoundRobinLoadBalancerTest { subchannels2.put(new EquivalentAddressGroup(oldAddr), oldSubchannel); subchannels2.put(new EquivalentAddressGroup(newAddr), newSubchannel); - List latestServers = Lists.newArrayList( - ResolvedServerInfoGroup.builder() - .add(new ResolvedServerInfo(oldAddr)) - .add(new ResolvedServerInfo(newAddr)) - .build()); + List latestServers = + Lists.newArrayList( + new EquivalentAddressGroup(oldAddr), + new EquivalentAddressGroup(newAddr)); - loadBalancer.handleResolvedAddresses(latestServers, affinity); + loadBalancer.handleResolvedAddressGroups(latestServers, affinity); verify(newSubchannel, times(1)).requestConnection(); verify(removedSubchannel, times(1)).shutdown(); @@ -253,7 +249,7 @@ public class RoundRobinLoadBalancerTest { @Test public void pickAfterStateChange() throws Exception { InOrder inOrder = inOrder(mockHelper); - loadBalancer.handleResolvedAddresses(Lists.newArrayList(servers.keySet()), Attributes.EMPTY); + loadBalancer.handleResolvedAddressGroups(servers, Attributes.EMPTY); Subchannel subchannel = loadBalancer.getSubchannels().iterator().next(); AtomicReference subchannelStateInfo = subchannel.getAttributes().get( STATE_INFO); @@ -329,7 +325,7 @@ public class RoundRobinLoadBalancerTest { @Test public void nameResolutionErrorWithActiveChannels() throws Exception { final Subchannel readySubchannel = subchannels.values().iterator().next(); - loadBalancer.handleResolvedAddresses(Lists.newArrayList(servers.keySet()), affinity); + loadBalancer.handleResolvedAddressGroups(servers, affinity); loadBalancer.handleSubchannelState(readySubchannel, ConnectivityStateInfo.forNonError(READY)); loadBalancer.handleNameResolutionError(Status.NOT_FOUND.withDescription("nameResolutionError")); @@ -353,7 +349,7 @@ public class RoundRobinLoadBalancerTest { Subchannel sc2 = subchannelIterator.next(); Subchannel sc3 = subchannelIterator.next(); - loadBalancer.handleResolvedAddresses(Lists.newArrayList(servers.keySet()), Attributes.EMPTY); + loadBalancer.handleResolvedAddressGroups(servers, Attributes.EMPTY); verify(sc1, times(1)).requestConnection(); verify(sc2, times(1)).requestConnection(); verify(sc3, 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 4158cda37f..e7c2c94c1a 100644 --- a/grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancer.java +++ b/grpclb/src/main/java/io/grpc/grpclb/GrpclbLoadBalancer.java @@ -48,7 +48,6 @@ import io.grpc.EquivalentAddressGroup; import io.grpc.LoadBalancer; import io.grpc.ManagedChannel; import io.grpc.Metadata; -import io.grpc.ResolvedServerInfoGroup; import io.grpc.Status; import io.grpc.grpclb.GrpclbConstants.LbPolicy; import io.grpc.internal.LogId; @@ -167,26 +166,23 @@ class GrpclbLoadBalancer extends LoadBalancer implements WithLogId { } @Override - public void handleResolvedAddresses(List updatedServers, - Attributes attributes) { + public void handleResolvedAddressGroups( + List updatedServers, Attributes attributes) { LbPolicy newLbPolicy = attributes.get(GrpclbConstants.ATTR_LB_POLICY); // LB addresses and backend addresses are treated separately List newLbAddressGroups = new ArrayList(); - List newBackendServerInfoGroups = - new ArrayList(); - for (ResolvedServerInfoGroup serverInfoGroup : updatedServers) { - String lbAddrAuthority = serverInfoGroup.getAttributes().get( - GrpclbConstants.ATTR_LB_ADDR_AUTHORITY); - EquivalentAddressGroup eag = serverInfoGroup.toEquivalentAddressGroup(); + List newBackendServers = new ArrayList(); + for (EquivalentAddressGroup server : updatedServers) { + String lbAddrAuthority = server.getAttributes().get(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY); if (lbAddrAuthority != null) { - newLbAddressGroups.add(new LbAddressGroup(eag, lbAddrAuthority)); + newLbAddressGroups.add(new LbAddressGroup(server, lbAddrAuthority)); } else { - newBackendServerInfoGroups.add(serverInfoGroup); + newBackendServers.add(server); } } - if (newBackendServerInfoGroups.isEmpty()) { - // handleResolvedAddresses()'s javadoc has guaranteed updatedServers is never empty. + if (newBackendServers.isEmpty()) { + // handleResolvedAddressGroups()'s javadoc has guaranteed updatedServers is never empty. checkState(!newLbAddressGroups.isEmpty(), "No backend address nor LB address. updatedServers=%s", updatedServers); if (newLbPolicy != LbPolicy.GRPCLB) { @@ -226,7 +222,7 @@ class GrpclbLoadBalancer extends LoadBalancer implements WithLogId { case PICK_FIRST: case ROUND_ROBIN: checkNotNull(delegate, "delegate should not be null. newLbPolicy=" + newLbPolicy); - delegate.handleResolvedAddresses(newBackendServerInfoGroups, attributes); + delegate.handleResolvedAddressGroups(newBackendServers, attributes); break; case GRPCLB: if (newLbAddressGroups.isEmpty()) { diff --git a/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java b/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java index 0c2e9e8d49..7b7544f9a9 100644 --- a/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java +++ b/grpclb/src/test/java/io/grpc/grpclb/GrpclbLoadBalancerTest.java @@ -73,8 +73,6 @@ import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.MethodDescriptor.Marshaller; -import io.grpc.ResolvedServerInfo; -import io.grpc.ResolvedServerInfoGroup; import io.grpc.Status; import io.grpc.StatusRuntimeException; import io.grpc.grpclb.GrpclbConstants.LbPolicy; @@ -338,13 +336,13 @@ public class GrpclbLoadBalancerTest { assertSame(error, errorPicker.result.getStatus()); // Recover with a subsequent success - List resolvedServers = createResolvedServerInfoGroupList(false); + List resolvedServers = createResolvedServerAddresses(false); Attributes resolutionAttrs = Attributes.newBuilder().set(RESOLUTION_ATTR, "yeah").build(); deliverResolvedAddresses(resolvedServers, resolutionAttrs); verify(pickFirstBalancerFactory).newLoadBalancer(helper); - verify(pickFirstBalancer).handleResolvedAddresses(eq(resolvedServers), eq(resolutionAttrs)); + verify(pickFirstBalancer).handleResolvedAddressGroups(eq(resolvedServers), eq(resolutionAttrs)); verifyNoMoreInteractions(roundRobinBalancerFactory); verifyNoMoreInteractions(roundRobinBalancer); } @@ -358,8 +356,8 @@ public class GrpclbLoadBalancerTest { assertSame(error, errorPicker.result.getStatus()); // Recover with a subsequent success - List resolvedServers = createResolvedServerInfoGroupList(true); - EquivalentAddressGroup eag = resolvedServers.get(0).toEquivalentAddressGroup(); + List resolvedServers = createResolvedServerAddresses(true); + EquivalentAddressGroup eag = resolvedServers.get(0); Attributes resolutionAttrs = Attributes.newBuilder() .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.GRPCLB).build(); @@ -378,13 +376,13 @@ public class GrpclbLoadBalancerTest { @Test public void delegatingPickFirstThenNameResolutionFails() { - List resolvedServers = createResolvedServerInfoGroupList(false); + List resolvedServers = createResolvedServerAddresses(false); Attributes resolutionAttrs = Attributes.newBuilder().set(RESOLUTION_ATTR, "yeah").build(); deliverResolvedAddresses(resolvedServers, resolutionAttrs); verify(pickFirstBalancerFactory).newLoadBalancer(helper); - verify(pickFirstBalancer).handleResolvedAddresses(eq(resolvedServers), eq(resolutionAttrs)); + verify(pickFirstBalancer).handleResolvedAddressGroups(eq(resolvedServers), eq(resolutionAttrs)); // Then let name resolution fail. The error will be passed directly to the delegate. Status error = Status.NOT_FOUND.withDescription("www.google.com not found"); @@ -397,7 +395,7 @@ public class GrpclbLoadBalancerTest { @Test public void delegatingRoundRobinThenNameResolutionFails() { - List resolvedServers = createResolvedServerInfoGroupList(false, false); + List resolvedServers = createResolvedServerAddresses(false, false); Attributes resolutionAttrs = Attributes.newBuilder() .set(RESOLUTION_ATTR, "yeah") @@ -406,7 +404,7 @@ public class GrpclbLoadBalancerTest { deliverResolvedAddresses(resolvedServers, resolutionAttrs); verify(roundRobinBalancerFactory).newLoadBalancer(helper); - verify(roundRobinBalancer).handleResolvedAddresses(resolvedServers, resolutionAttrs); + verify(roundRobinBalancer).handleResolvedAddressGroups(resolvedServers, resolutionAttrs); // Then let name resolution fail. The error will be passed directly to the delegate. Status error = Status.NOT_FOUND.withDescription("www.google.com not found"); @@ -421,16 +419,14 @@ public class GrpclbLoadBalancerTest { public void grpclbThenNameResolutionFails() { InOrder inOrder = inOrder(helper); // Go to GRPCLB first - List grpclbResolutionList = - createResolvedServerInfoGroupList(true, true); + List grpclbResolutionList = createResolvedServerAddresses(true, true); Attributes grpclbResolutionAttrs = Attributes.newBuilder() .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.GRPCLB).build(); deliverResolvedAddresses(grpclbResolutionList, grpclbResolutionAttrs); assertSame(LbPolicy.GRPCLB, balancer.getLbPolicy()); assertNull(balancer.getDelegate()); - verify(helper).createOobChannel(eq(grpclbResolutionList.get(0).toEquivalentAddressGroup()), - eq(lbAuthority(0))); + verify(helper).createOobChannel(eq(grpclbResolutionList.get(0)), eq(lbAuthority(0))); assertEquals(1, fakeOobChannels.size()); ManagedChannel oobChannel = fakeOobChannels.poll(); verify(mockLbService).balanceLoad(lbResponseObserverCaptor.capture()); @@ -464,23 +460,22 @@ public class GrpclbLoadBalancerTest { @Test public void switchPolicy() { // Go to GRPCLB first - List grpclbResolutionList = - createResolvedServerInfoGroupList(true, false, true); + List grpclbResolutionList = + createResolvedServerAddresses(true, false, true); Attributes grpclbResolutionAttrs = Attributes.newBuilder() .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.GRPCLB).build(); deliverResolvedAddresses(grpclbResolutionList, grpclbResolutionAttrs); assertSame(LbPolicy.GRPCLB, balancer.getLbPolicy()); assertNull(balancer.getDelegate()); - verify(helper).createOobChannel(eq(grpclbResolutionList.get(0).toEquivalentAddressGroup()), - eq(lbAuthority(0))); + verify(helper).createOobChannel(eq(grpclbResolutionList.get(0)), eq(lbAuthority(0))); assertEquals(1, fakeOobChannels.size()); ManagedChannel oobChannel = fakeOobChannels.poll(); verify(mockLbService).balanceLoad(lbResponseObserverCaptor.capture()); // Switch to PICK_FIRST - List pickFirstResolutionList = - createResolvedServerInfoGroupList(true, false, true); + List pickFirstResolutionList = + createResolvedServerAddresses(true, false, true); Attributes pickFirstResolutionAttrs = Attributes.newBuilder() .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.PICK_FIRST).build(); verify(pickFirstBalancerFactory, never()).newLoadBalancer(any(Helper.class)); @@ -493,7 +488,7 @@ public class GrpclbLoadBalancerTest { verify(pickFirstBalancerFactory).newLoadBalancer(same(helper)); // Only non-LB addresses are passed to the delegate - verify(pickFirstBalancer).handleResolvedAddresses( + verify(pickFirstBalancer).handleResolvedAddressGroups( eq(Arrays.asList(pickFirstResolutionList.get(1))), same(pickFirstResolutionAttrs)); assertSame(LbPolicy.PICK_FIRST, balancer.getLbPolicy()); assertSame(pickFirstBalancer, balancer.getDelegate()); @@ -502,8 +497,8 @@ public class GrpclbLoadBalancerTest { assertTrue(oobChannel.isShutdown()); // Switch to ROUND_ROBIN - List roundRobinResolutionList = - createResolvedServerInfoGroupList(true, false, false); + List roundRobinResolutionList = + createResolvedServerAddresses(true, false, false); Attributes roundRobinResolutionAttrs = Attributes.newBuilder() .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.ROUND_ROBIN).build(); verify(roundRobinBalancerFactory, never()).newLoadBalancer(any(Helper.class)); @@ -511,30 +506,28 @@ public class GrpclbLoadBalancerTest { verify(roundRobinBalancerFactory).newLoadBalancer(same(helper)); // Only non-LB addresses are passed to the delegate - verify(roundRobinBalancer).handleResolvedAddresses( + verify(roundRobinBalancer).handleResolvedAddressGroups( eq(roundRobinResolutionList.subList(1, 3)), same(roundRobinResolutionAttrs)); assertSame(LbPolicy.ROUND_ROBIN, balancer.getLbPolicy()); assertSame(roundRobinBalancer, balancer.getDelegate()); // Special case: if all addresses are loadbalancers, use GRPCLB no matter what the NameResolver // says. - grpclbResolutionList = createResolvedServerInfoGroupList(true, true, true); + grpclbResolutionList = createResolvedServerAddresses(true, true, true); grpclbResolutionAttrs = Attributes.newBuilder() .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.PICK_FIRST).build(); deliverResolvedAddresses(grpclbResolutionList, grpclbResolutionAttrs); assertSame(LbPolicy.GRPCLB, balancer.getLbPolicy()); assertNull(balancer.getDelegate()); - verify(helper, times(2)).createOobChannel( - eq(grpclbResolutionList.get(0).toEquivalentAddressGroup()), - eq(lbAuthority(0))); + verify(helper, times(2)).createOobChannel(eq(grpclbResolutionList.get(0)), eq(lbAuthority(0))); verify(helper, times(2)).createOobChannel(any(EquivalentAddressGroup.class), any(String.class)); assertEquals(1, fakeOobChannels.size()); oobChannel = fakeOobChannels.poll(); verify(mockLbService, times(2)).balanceLoad(lbResponseObserverCaptor.capture()); // Special case: PICK_FIRST is the default - pickFirstResolutionList = createResolvedServerInfoGroupList(true, false, false); + pickFirstResolutionList = createResolvedServerAddresses(true, false, false); pickFirstResolutionAttrs = Attributes.EMPTY; verify(pickFirstBalancerFactory).newLoadBalancer(any(Helper.class)); assertFalse(oobChannel.isShutdown()); @@ -542,7 +535,7 @@ public class GrpclbLoadBalancerTest { verify(pickFirstBalancerFactory, times(2)).newLoadBalancer(same(helper)); // Only non-LB addresses are passed to the delegate - verify(pickFirstBalancer).handleResolvedAddresses( + verify(pickFirstBalancer).handleResolvedAddressGroups( eq(pickFirstResolutionList.subList(1, 3)), same(pickFirstResolutionAttrs)); assertSame(LbPolicy.PICK_FIRST, balancer.getLbPolicy()); assertSame(pickFirstBalancer, balancer.getDelegate()); @@ -553,16 +546,14 @@ public class GrpclbLoadBalancerTest { @Test public void grpclbWorking() { InOrder inOrder = inOrder(helper); - List grpclbResolutionList = - createResolvedServerInfoGroupList(true, true); + List grpclbResolutionList = createResolvedServerAddresses(true, true); Attributes grpclbResolutionAttrs = Attributes.newBuilder() .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.GRPCLB).build(); deliverResolvedAddresses(grpclbResolutionList, grpclbResolutionAttrs); assertSame(LbPolicy.GRPCLB, balancer.getLbPolicy()); assertNull(balancer.getDelegate()); - verify(helper).createOobChannel(eq(grpclbResolutionList.get(0).toEquivalentAddressGroup()), - eq(lbAuthority(0))); + verify(helper).createOobChannel(eq(grpclbResolutionList.get(0)), eq(lbAuthority(0))); assertEquals(1, fakeOobChannels.size()); ManagedChannel oobChannel = fakeOobChannels.poll(); verify(mockLbService).balanceLoad(lbResponseObserverCaptor.capture()); @@ -700,25 +691,21 @@ public class GrpclbLoadBalancerTest { InOrder inOrder = inOrder(helper, mockLbService); // Make the first LB address fail to connect failingLbAuthorities.add(lbAuthority(0)); - List grpclbResolutionList = - createResolvedServerInfoGroupList(true, true, true); + List grpclbResolutionList = + createResolvedServerAddresses(true, true, true); Attributes grpclbResolutionAttrs = Attributes.newBuilder() .set(GrpclbConstants.ATTR_LB_POLICY, LbPolicy.GRPCLB).build(); deliverResolvedAddresses(grpclbResolutionList, grpclbResolutionAttrs); // First LB addr fails to connect - inOrder.verify(helper).createOobChannel( - eq(grpclbResolutionList.get(0).toEquivalentAddressGroup()), - eq(lbAuthority(0))); + inOrder.verify(helper).createOobChannel(eq(grpclbResolutionList.get(0)), eq(lbAuthority(0))); inOrder.verify(helper).updatePicker(isA(ErrorPicker.class)); assertEquals(2, fakeOobChannels.size()); assertTrue(fakeOobChannels.poll().isShutdown()); // Will move on to second LB addr - inOrder.verify(helper).createOobChannel( - eq(grpclbResolutionList.get(1).toEquivalentAddressGroup()), - eq(lbAuthority(1))); + inOrder.verify(helper).createOobChannel(eq(grpclbResolutionList.get(1)), eq(lbAuthority(1))); inOrder.verify(mockLbService).balanceLoad(lbResponseObserverCaptor.capture()); StreamObserver lbResponseObserver = lbResponseObserverCaptor.getValue(); assertEquals(1, lbRequestObservers.size()); @@ -736,9 +723,7 @@ public class GrpclbLoadBalancerTest { assertEquals(error1.getCode(), errorPicker.result.getStatus().getCode()); assertTrue(errorPicker.result.getStatus().getDescription().contains(error1.getDescription())); // Move on to the third LB. - inOrder.verify(helper).createOobChannel( - eq(grpclbResolutionList.get(2).toEquivalentAddressGroup()), - eq(lbAuthority(2))); + inOrder.verify(helper).createOobChannel(eq(grpclbResolutionList.get(2)), eq(lbAuthority(2))); inOrder.verify(mockLbService).balanceLoad(lbResponseObserverCaptor.capture()); lbResponseObserver = lbResponseObserverCaptor.getValue(); @@ -753,17 +738,13 @@ public class GrpclbLoadBalancerTest { assertTrue(fakeOobChannels.poll().isShutdown()); // Loop back to the first LB addr, which still fails. - inOrder.verify(helper).createOobChannel( - eq(grpclbResolutionList.get(0).toEquivalentAddressGroup()), - eq(lbAuthority(0))); + inOrder.verify(helper).createOobChannel(eq(grpclbResolutionList.get(0)), eq(lbAuthority(0))); inOrder.verify(helper).updatePicker(isA(ErrorPicker.class)); assertEquals(2, fakeOobChannels.size()); assertTrue(fakeOobChannels.poll().isShutdown()); // Will move on to second LB addr - inOrder.verify(helper).createOobChannel( - eq(grpclbResolutionList.get(1).toEquivalentAddressGroup()), - eq(lbAuthority(1))); + inOrder.verify(helper).createOobChannel(eq(grpclbResolutionList.get(1)), eq(lbAuthority(1))); inOrder.verify(mockLbService).balanceLoad(lbResponseObserverCaptor.capture()); lbResponseObserver = lbResponseObserverCaptor.getValue(); assertEquals(1, lbRequestObservers.size()); @@ -805,27 +786,26 @@ public class GrpclbLoadBalancerTest { } private void deliverResolvedAddresses( - final List addrs, final Attributes attrs) { + final List addrs, final Attributes attrs) { channelExecutor.execute(new Runnable() { @Override public void run() { - balancer.handleResolvedAddresses(addrs, attrs); + balancer.handleResolvedAddressGroups(addrs, attrs); } }); } - private static List createResolvedServerInfoGroupList(boolean ... isLb) { - ArrayList list = new ArrayList(); + private static List createResolvedServerAddresses(boolean ... isLb) { + ArrayList list = new ArrayList(); for (int i = 0; i < isLb.length; i++) { SocketAddress addr = new FakeSocketAddress("fake-address-" + i); - ResolvedServerInfoGroup serverInfoGroup = ResolvedServerInfoGroup - .builder(isLb[i] ? Attributes.newBuilder() - .set(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY, lbAuthority(i)) - .build() - : Attributes.EMPTY) - .add(new ResolvedServerInfo(addr)) - .build(); - list.add(serverInfoGroup); + EquivalentAddressGroup eag = + new EquivalentAddressGroup( + addr, + isLb[i] ? Attributes.newBuilder() + .set(GrpclbConstants.ATTR_LB_ADDR_AUTHORITY, lbAuthority(i)) + .build() : Attributes.EMPTY); + list.add(eag); } return list; }