From ddb2e52b4f7e32685f0bfcda1b9e7e6b92b5a4c4 Mon Sep 17 00:00:00 2001 From: Jihun Cho Date: Tue, 11 Feb 2020 10:07:42 -0800 Subject: [PATCH] core: revert stickiness from round robin --- .../io/grpc/internal/ServiceConfigUtil.java | 9 - .../io/grpc/util/RoundRobinLoadBalancer.java | 160 +------- .../grpc/util/RoundRobinLoadBalancerTest.java | 382 +----------------- 3 files changed, 13 insertions(+), 538 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java b/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java index e677bb6f1af..a7dbfbd673f 100644 --- a/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java +++ b/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java @@ -314,15 +314,6 @@ public static List unwrapLoadBalancingConfigList(List> return Collections.unmodifiableList(result); } - /** - * Extracts the stickiness metadata key from a service config, or {@code null}. - */ - @Nullable - public static String getStickinessMetadataKeyFromServiceConfig( - Map serviceConfig) { - return JsonUtil.getString(serviceConfig, "stickinessMetadataKey"); - } - /** * A LoadBalancingConfig that includes the policy name (the key) and its raw config value (parsed * JSON). diff --git a/core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java b/core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java index 82d803a8294..c8cb2084b3b 100644 --- a/core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java +++ b/core/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java @@ -27,20 +27,13 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.base.Preconditions; - 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.SubchannelStateListener; -import io.grpc.Metadata; -import io.grpc.Metadata.Key; import io.grpc.NameResolver; import io.grpc.Status; -import io.grpc.internal.GrpcAttributes; -import io.grpc.internal.ServiceConfigUtil; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -48,15 +41,10 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Queue; import java.util.Random; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import javax.annotation.Nonnull; -import javax.annotation.Nullable; /** * A {@link LoadBalancer} that provides round-robin load-balancing over the {@link @@ -66,8 +54,6 @@ final class RoundRobinLoadBalancer extends LoadBalancer { @VisibleForTesting static final Attributes.Key> STATE_INFO = Attributes.Key.create("state-info"); - // package-private to avoid synthetic access - static final Attributes.Key> STICKY_REF = Attributes.Key.create("sticky-ref"); private final Helper helper; private final Map subchannels = @@ -77,40 +63,18 @@ final class RoundRobinLoadBalancer extends LoadBalancer { private ConnectivityState currentState; private RoundRobinPicker currentPicker = new EmptyPicker(EMPTY_OK); - @Nullable - private StickinessState stickinessState; - RoundRobinLoadBalancer(Helper helper) { this.helper = checkNotNull(helper, "helper"); this.random = new Random(); } @Override - @SuppressWarnings("deprecation") public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { List servers = resolvedAddresses.getAddresses(); - Attributes attributes = resolvedAddresses.getAttributes(); Set currentAddrs = subchannels.keySet(); Map latestAddrs = stripAttrs(servers); Set removedAddrs = setsDifference(currentAddrs, latestAddrs.keySet()); - Map serviceConfig = attributes.get(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG); - if (serviceConfig != null) { - String stickinessMetadataKey = - ServiceConfigUtil.getStickinessMetadataKeyFromServiceConfig(serviceConfig); - if (stickinessMetadataKey != null) { - if (stickinessMetadataKey.endsWith(Metadata.BINARY_HEADER_SUFFIX)) { - helper.getChannelLogger().log( - ChannelLogLevel.WARNING, - "Binary stickiness header is not supported. The header \"{0}\" will be ignored", - stickinessMetadataKey); - } else if (stickinessState == null - || !stickinessState.key.name().equals(stickinessMetadataKey)) { - stickinessState = new StickinessState(stickinessMetadataKey); - } - } - } - for (Map.Entry latestEntry : latestAddrs.entrySet()) { EquivalentAddressGroup strippedAddressGroup = latestEntry.getKey(); @@ -133,11 +97,6 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { .set(STATE_INFO, new Ref<>(ConnectivityStateInfo.forNonError(IDLE))); - Ref stickyRef = null; - if (stickinessState != null) { - subchannelAttrs.set(STICKY_REF, stickyRef = new Ref<>(null)); - } - final Subchannel subchannel = checkNotNull( helper.createSubchannel(CreateSubchannelArgs.newBuilder() .setAddresses(originalAddressGroup) @@ -150,9 +109,6 @@ public void onSubchannelState(ConnectivityStateInfo state) { processSubchannelState(subchannel, state); } }); - if (stickyRef != null) { - stickyRef.value = subchannel; - } subchannels.put(strippedAddressGroup, subchannel); subchannel.requestConnection(); } @@ -183,9 +139,6 @@ private void processSubchannelState(Subchannel subchannel, ConnectivityStateInfo if (subchannels.get(stripAttrs(subchannel.getAddresses())) != subchannel) { return; } - if (stateInfo.getState() == SHUTDOWN && stickinessState != null) { - stickinessState.remove(subchannel); - } if (stateInfo.getState() == IDLE) { subchannel.requestConnection(); } @@ -197,9 +150,6 @@ private void shutdownSubchannel(Subchannel subchannel) { subchannel.shutdown(); getSubchannelStateInfoRef(subchannel).value = ConnectivityStateInfo.forNonError(SHUTDOWN); - if (stickinessState != null) { - stickinessState.remove(subchannel); - } } @Override @@ -241,7 +191,7 @@ private void updateBalancingState() { // initialize the Picker to a random start index to ensure that a high frequency of Picker // churn does not skew subchannel selection. int startIndex = random.nextInt(activeList.size()); - updateBalancingState(READY, new ReadyPicker(activeList, startIndex, stickinessState)); + updateBalancingState(READY, new ReadyPicker(activeList, startIndex)); } } @@ -305,90 +255,6 @@ private static Set setsDifference(Set a, Set b) { return aCopy; } - Map> getStickinessMapForTest() { - if (stickinessState == null) { - return null; - } - return stickinessState.stickinessMap; - } - - /** - * Holds stickiness related states: The stickiness key, a registry mapping stickiness values to - * the associated Subchannel Ref, and a map from Subchannel to Subchannel Ref. - */ - @VisibleForTesting - static final class StickinessState { - static final int MAX_ENTRIES = 1000; - - final Key key; - final ConcurrentMap> stickinessMap = - new ConcurrentHashMap<>(); - - final Queue evictionQueue = new ConcurrentLinkedQueue<>(); - - StickinessState(@Nonnull String stickinessKey) { - this.key = Key.of(stickinessKey, Metadata.ASCII_STRING_MARSHALLER); - } - - /** - * Returns the subchannel associated to the stickiness value if available in both the - * registry and the round robin list, otherwise associates the given subchannel with the - * stickiness key in the registry and returns the given subchannel. - */ - @Nonnull - Subchannel maybeRegister( - String stickinessValue, @Nonnull Subchannel subchannel) { - final Ref newSubchannelRef = subchannel.getAttributes().get(STICKY_REF); - while (true) { - Ref existingSubchannelRef = - stickinessMap.putIfAbsent(stickinessValue, newSubchannelRef); - if (existingSubchannelRef == null) { - // new entry - addToEvictionQueue(stickinessValue); - return subchannel; - } else { - // existing entry - Subchannel existingSubchannel = existingSubchannelRef.value; - if (existingSubchannel != null && isReady(existingSubchannel)) { - return existingSubchannel; - } - } - // existingSubchannelRef is not null but no longer valid, replace it - if (stickinessMap.replace(stickinessValue, existingSubchannelRef, newSubchannelRef)) { - return subchannel; - } - // another thread concurrently removed or updated the entry, try again - } - } - - private void addToEvictionQueue(String value) { - String oldValue; - while (stickinessMap.size() >= MAX_ENTRIES && (oldValue = evictionQueue.poll()) != null) { - stickinessMap.remove(oldValue); - } - evictionQueue.add(value); - } - - /** - * Unregister the subchannel from StickinessState. - */ - void remove(Subchannel subchannel) { - subchannel.getAttributes().get(STICKY_REF).value = null; - } - - /** - * Gets the subchannel associated with the stickiness value if there is. - */ - @Nullable - Subchannel getSubchannel(String stickinessValue) { - Ref subchannelRef = stickinessMap.get(stickinessValue); - if (subchannelRef != null) { - return subchannelRef.value; - } - return null; - } - } - // Only subclasses are ReadyPicker or EmptyPicker private abstract static class RoundRobinPicker extends SubchannelPicker { abstract boolean isEquivalentTo(RoundRobinPicker picker); @@ -400,33 +266,18 @@ static final class ReadyPicker extends RoundRobinPicker { AtomicIntegerFieldUpdater.newUpdater(ReadyPicker.class, "index"); private final List list; // non-empty - @Nullable - private final RoundRobinLoadBalancer.StickinessState stickinessState; @SuppressWarnings("unused") private volatile int index; - ReadyPicker(List list, int startIndex, - @Nullable RoundRobinLoadBalancer.StickinessState stickinessState) { + ReadyPicker(List list, int startIndex) { Preconditions.checkArgument(!list.isEmpty(), "empty list"); this.list = list; - this.stickinessState = stickinessState; this.index = startIndex - 1; } @Override public PickResult pickSubchannel(PickSubchannelArgs args) { - Subchannel subchannel = null; - if (stickinessState != null) { - String stickinessValue = args.getHeaders().get(stickinessState.key); - if (stickinessValue != null) { - subchannel = stickinessState.getSubchannel(stickinessValue); - if (subchannel == null || !RoundRobinLoadBalancer.isReady(subchannel)) { - subchannel = stickinessState.maybeRegister(stickinessValue, nextSubchannel()); - } - } - } - - return PickResult.withSubchannel(subchannel != null ? subchannel : nextSubchannel()); + return PickResult.withSubchannel(nextSubchannel()); } @Override @@ -457,9 +308,8 @@ boolean isEquivalentTo(RoundRobinPicker picker) { } ReadyPicker other = (ReadyPicker) picker; // the lists cannot contain duplicate subchannels - return other == this || (stickinessState == other.stickinessState - && list.size() == other.list.size() - && new HashSet<>(list).containsAll(other.list)); + return other == this + || (list.size() == other.list.size() && new HashSet<>(list).containsAll(other.list)); } } diff --git a/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java b/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java index a682ddd1ffd..4346accf709 100644 --- a/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java +++ b/core/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java @@ -25,17 +25,12 @@ import static io.grpc.util.RoundRobinLoadBalancer.STATE_INFO; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isA; -import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -59,19 +54,14 @@ import io.grpc.LoadBalancer.Subchannel; import io.grpc.LoadBalancer.SubchannelPicker; import io.grpc.LoadBalancer.SubchannelStateListener; -import io.grpc.Metadata; -import io.grpc.Metadata.Key; import io.grpc.Status; -import io.grpc.internal.GrpcAttributes; import io.grpc.util.RoundRobinLoadBalancer.EmptyPicker; import io.grpc.util.RoundRobinLoadBalancer.ReadyPicker; import io.grpc.util.RoundRobinLoadBalancer.Ref; -import io.grpc.util.RoundRobinLoadBalancer.StickinessState; import java.net.SocketAddress; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -115,7 +105,6 @@ public class RoundRobinLoadBalancerTest { private PickSubchannelArgs mockArgs; @Before - @SuppressWarnings("unchecked") public void setUp() { MockitoAnnotations.initMocks(this); @@ -188,7 +177,6 @@ public void pickAfterResolved() throws Exception { verifyNoMoreInteractions(mockHelper); } - @SuppressWarnings("unchecked") @Test public void pickAfterResolvedUpdatedHosts() throws Exception { Subchannel removedSubchannel = mock(Subchannel.class); @@ -268,7 +256,6 @@ public void pickAfterResolvedUpdatedHosts() throws Exception { verifyNoMoreInteractions(mockHelper); } - @SuppressWarnings("unchecked") @Test public void pickAfterStateChange() throws Exception { InOrder inOrder = inOrder(mockHelper); @@ -307,10 +294,6 @@ public void pickAfterStateChange() throws Exception { verifyNoMoreInteractions(mockHelper); } - private Subchannel nextSubchannel(Subchannel current, List allSubChannels) { - return allSubChannels.get((allSubChannels.indexOf(current) + 1) % allSubChannels.size()); - } - @Test public void pickerRoundRobin() throws Exception { Subchannel subchannel = mock(Subchannel.class); @@ -319,7 +302,7 @@ public void pickerRoundRobin() throws Exception { ReadyPicker picker = new ReadyPicker(Collections.unmodifiableList( Lists.newArrayList(subchannel, subchannel1, subchannel2)), - 0 /* startIndex */, null /* stickinessState */); + 0 /* startIndex */); assertThat(picker.getList()).containsExactly(subchannel, subchannel1, subchannel2); @@ -349,7 +332,6 @@ public void nameResolutionErrorWithNoChannels() throws Exception { verifyNoMoreInteractions(mockHelper); } - @SuppressWarnings("unchecked") @Test public void nameResolutionErrorWithActiveChannels() throws Exception { final Subchannel readySubchannel = subchannels.values().iterator().next(); @@ -421,356 +403,10 @@ public void subchannelStateIsolation() throws Exception { assertThat(pickers.hasNext()).isFalse(); } - @Test - public void noStickinessEnabled_withStickyHeader() { - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(Attributes.EMPTY) - .build()); - for (Subchannel subchannel : subchannels.values()) { - deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(READY)); - } - verify(mockHelper, times(4)) - .updateBalancingState(any(ConnectivityState.class), pickerCaptor.capture()); - SubchannelPicker picker = pickerCaptor.getValue(); - - Key stickinessKey = Key.of("my-sticky-key", Metadata.ASCII_STRING_MARSHALLER); - Metadata headerWithStickinessValue = new Metadata(); - headerWithStickinessValue.put(stickinessKey, "my-sticky-value"); - doReturn(headerWithStickinessValue).when(mockArgs).getHeaders(); - - List allSubchannels = getList(picker); - Subchannel sc1 = picker.pickSubchannel(mockArgs).getSubchannel(); - Subchannel sc2 = picker.pickSubchannel(mockArgs).getSubchannel(); - Subchannel sc3 = picker.pickSubchannel(mockArgs).getSubchannel(); - Subchannel sc4 = picker.pickSubchannel(mockArgs).getSubchannel(); - - assertEquals(nextSubchannel(sc1, allSubchannels), sc2); - assertEquals(nextSubchannel(sc2, allSubchannels), sc3); - assertEquals(nextSubchannel(sc3, allSubchannels), sc1); - assertEquals(sc4, sc1); - - assertNull(loadBalancer.getStickinessMapForTest()); - } - - @Test - @SuppressWarnings("deprecation") // migrate to parsed object - public void stickinessEnabled_withoutStickyHeader() { - Map serviceConfig = new HashMap<>(); - serviceConfig.put("stickinessMetadataKey", "my-sticky-key"); - Attributes attributes = Attributes.newBuilder() - .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig).build(); - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes).build()); - for (Subchannel subchannel : subchannels.values()) { - deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(READY)); - } - verify(mockHelper, times(4)) - .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture()); - SubchannelPicker picker = pickerCaptor.getValue(); - - doReturn(new Metadata()).when(mockArgs).getHeaders(); - - List allSubchannels = getList(picker); - - Subchannel sc1 = picker.pickSubchannel(mockArgs).getSubchannel(); - Subchannel sc2 = picker.pickSubchannel(mockArgs).getSubchannel(); - Subchannel sc3 = picker.pickSubchannel(mockArgs).getSubchannel(); - Subchannel sc4 = picker.pickSubchannel(mockArgs).getSubchannel(); - - assertEquals(nextSubchannel(sc1, allSubchannels), sc2); - assertEquals(nextSubchannel(sc2, allSubchannels), sc3); - assertEquals(nextSubchannel(sc3, allSubchannels), sc1); - assertEquals(sc4, sc1); - verify(mockArgs, times(4)).getHeaders(); - assertNotNull(loadBalancer.getStickinessMapForTest()); - assertThat(loadBalancer.getStickinessMapForTest()).isEmpty(); - } - - @Test - @SuppressWarnings("deprecation") // migrate to parsed object - public void stickinessEnabled_withStickyHeader() { - Map serviceConfig = new HashMap<>(); - serviceConfig.put("stickinessMetadataKey", "my-sticky-key"); - Attributes attributes = Attributes.newBuilder() - .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig).build(); - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes).build()); - for (Subchannel subchannel : subchannels.values()) { - deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(READY)); - } - verify(mockHelper, times(4)) - .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture()); - SubchannelPicker picker = pickerCaptor.getValue(); - - Key stickinessKey = Key.of("my-sticky-key", Metadata.ASCII_STRING_MARSHALLER); - Metadata headerWithStickinessValue = new Metadata(); - headerWithStickinessValue.put(stickinessKey, "my-sticky-value"); - doReturn(headerWithStickinessValue).when(mockArgs).getHeaders(); - - Subchannel sc1 = picker.pickSubchannel(mockArgs).getSubchannel(); - assertEquals(sc1, picker.pickSubchannel(mockArgs).getSubchannel()); - assertEquals(sc1, picker.pickSubchannel(mockArgs).getSubchannel()); - assertEquals(sc1, picker.pickSubchannel(mockArgs).getSubchannel()); - assertEquals(sc1, picker.pickSubchannel(mockArgs).getSubchannel()); - - verify(mockArgs, atLeast(4)).getHeaders(); - assertNotNull(loadBalancer.getStickinessMapForTest()); - assertThat(loadBalancer.getStickinessMapForTest()).hasSize(1); - } - - @Test - @SuppressWarnings("deprecation") // migrate to parsed object - public void stickinessEnabled_withDifferentStickyHeaders() { - Map serviceConfig = new HashMap<>(); - serviceConfig.put("stickinessMetadataKey", "my-sticky-key"); - Attributes attributes = Attributes.newBuilder() - .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig).build(); - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes).build()); - for (Subchannel subchannel : subchannels.values()) { - deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(READY)); - } - verify(mockHelper, times(4)) - .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture()); - SubchannelPicker picker = pickerCaptor.getValue(); - - Key stickinessKey = Key.of("my-sticky-key", Metadata.ASCII_STRING_MARSHALLER); - Metadata headerWithStickinessValue1 = new Metadata(); - headerWithStickinessValue1.put(stickinessKey, "my-sticky-value"); - - Metadata headerWithStickinessValue2 = new Metadata(); - headerWithStickinessValue2.put(stickinessKey, "my-sticky-value2"); - - List allSubchannels = getList(picker); - - doReturn(headerWithStickinessValue1).when(mockArgs).getHeaders(); - Subchannel sc1a = picker.pickSubchannel(mockArgs).getSubchannel(); - - doReturn(headerWithStickinessValue2).when(mockArgs).getHeaders(); - Subchannel sc2a = picker.pickSubchannel(mockArgs).getSubchannel(); - - doReturn(headerWithStickinessValue1).when(mockArgs).getHeaders(); - Subchannel sc1b = picker.pickSubchannel(mockArgs).getSubchannel(); - - doReturn(headerWithStickinessValue2).when(mockArgs).getHeaders(); - Subchannel sc2b = picker.pickSubchannel(mockArgs).getSubchannel(); - - assertEquals(sc1a, sc1b); - assertEquals(sc2a, sc2b); - assertEquals(nextSubchannel(sc1a, allSubchannels), sc2a); - assertEquals(nextSubchannel(sc1b, allSubchannels), sc2b); - - verify(mockArgs, atLeast(4)).getHeaders(); - assertNotNull(loadBalancer.getStickinessMapForTest()); - assertThat(loadBalancer.getStickinessMapForTest()).hasSize(2); - } - - @Test - @SuppressWarnings("deprecation") // migrate to parsed object - public void stickiness_goToTransientFailure_pick_backToReady() { - Map serviceConfig = new HashMap<>(); - serviceConfig.put("stickinessMetadataKey", "my-sticky-key"); - Attributes attributes = Attributes.newBuilder() - .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig).build(); - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes).build()); - for (Subchannel subchannel : subchannels.values()) { - deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(READY)); - } - verify(mockHelper, times(4)) - .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture()); - SubchannelPicker picker = pickerCaptor.getValue(); - - Key stickinessKey = Key.of("my-sticky-key", Metadata.ASCII_STRING_MARSHALLER); - Metadata headerWithStickinessValue = new Metadata(); - headerWithStickinessValue.put(stickinessKey, "my-sticky-value"); - doReturn(headerWithStickinessValue).when(mockArgs).getHeaders(); - - // first pick - Subchannel sc1 = picker.pickSubchannel(mockArgs).getSubchannel(); - - // go to transient failure - deliverSubchannelState(sc1, ConnectivityStateInfo.forTransientFailure(Status.UNAVAILABLE)); - - verify(mockHelper, times(5)) - .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture()); - picker = pickerCaptor.getValue(); - - // second pick - Subchannel sc2 = picker.pickSubchannel(mockArgs).getSubchannel(); - - // go back to ready - deliverSubchannelState(sc1, ConnectivityStateInfo.forNonError(READY)); - - verify(mockHelper, times(6)) - .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture()); - picker = pickerCaptor.getValue(); - - // third pick - Subchannel sc3 = picker.pickSubchannel(mockArgs).getSubchannel(); - assertEquals(sc2, sc3); - verify(mockArgs, atLeast(3)).getHeaders(); - assertNotNull(loadBalancer.getStickinessMapForTest()); - assertThat(loadBalancer.getStickinessMapForTest()).hasSize(1); - } - - @Test - @SuppressWarnings("deprecation") // migrate to parsed object - public void stickiness_goToTransientFailure_backToReady_pick() { - Map serviceConfig = new HashMap<>(); - serviceConfig.put("stickinessMetadataKey", "my-sticky-key"); - Attributes attributes = Attributes.newBuilder() - .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig).build(); - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes).build()); - for (Subchannel subchannel : subchannels.values()) { - deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(READY)); - } - verify(mockHelper, times(4)) - .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture()); - SubchannelPicker picker = pickerCaptor.getValue(); - - Key stickinessKey = Key.of("my-sticky-key", Metadata.ASCII_STRING_MARSHALLER); - Metadata headerWithStickinessValue1 = new Metadata(); - headerWithStickinessValue1.put(stickinessKey, "my-sticky-value"); - doReturn(headerWithStickinessValue1).when(mockArgs).getHeaders(); - - // first pick - Subchannel sc1 = picker.pickSubchannel(mockArgs).getSubchannel(); - - // go to transient failure - deliverSubchannelState(sc1, ConnectivityStateInfo.forTransientFailure(Status.UNAVAILABLE)); - - Metadata headerWithStickinessValue2 = new Metadata(); - headerWithStickinessValue2.put(stickinessKey, "my-sticky-value2"); - doReturn(headerWithStickinessValue2).when(mockArgs).getHeaders(); - verify(mockHelper, times(5)) - .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture()); - picker = pickerCaptor.getValue(); - - // second pick with a different stickiness value - @SuppressWarnings("unused") - Subchannel sc2 = picker.pickSubchannel(mockArgs).getSubchannel(); - - // go back to ready - deliverSubchannelState(sc1, ConnectivityStateInfo.forNonError(READY)); - - doReturn(headerWithStickinessValue1).when(mockArgs).getHeaders(); - verify(mockHelper, times(6)) - .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture()); - picker = pickerCaptor.getValue(); - - // third pick with my-sticky-value1 - Subchannel sc3 = picker.pickSubchannel(mockArgs).getSubchannel(); - assertEquals(sc1, sc3); - - verify(mockArgs, atLeast(3)).getHeaders(); - assertNotNull(loadBalancer.getStickinessMapForTest()); - assertThat(loadBalancer.getStickinessMapForTest()).hasSize(2); - } - - @Test - @SuppressWarnings("deprecation") // migrate to parsed object - public void stickiness_oneSubchannelShutdown() { - Map serviceConfig = new HashMap<>(); - serviceConfig.put("stickinessMetadataKey", "my-sticky-key"); - Attributes attributes = Attributes.newBuilder() - .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig).build(); - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes).build()); - for (Subchannel subchannel : subchannels.values()) { - deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(READY)); - } - verify(mockHelper, times(4)) - .updateBalancingState(stateCaptor.capture(), pickerCaptor.capture()); - SubchannelPicker picker = pickerCaptor.getValue(); - - Key stickinessKey = Key.of("my-sticky-key", Metadata.ASCII_STRING_MARSHALLER); - Metadata headerWithStickinessValue = new Metadata(); - headerWithStickinessValue.put(stickinessKey, "my-sticky-value"); - doReturn(headerWithStickinessValue).when(mockArgs).getHeaders(); - - List allSubchannels = Lists.newArrayList(getList(picker)); - - Subchannel sc1 = picker.pickSubchannel(mockArgs).getSubchannel(); - - // shutdown channel directly - deliverSubchannelState(sc1, ConnectivityStateInfo.forNonError(ConnectivityState.SHUTDOWN)); - - assertNull(loadBalancer.getStickinessMapForTest().get("my-sticky-value").value); - - assertEquals(nextSubchannel(sc1, allSubchannels), - picker.pickSubchannel(mockArgs).getSubchannel()); - assertThat(loadBalancer.getStickinessMapForTest()).hasSize(1); - verify(mockArgs, atLeast(2)).getHeaders(); - - Subchannel sc2 = picker.pickSubchannel(mockArgs).getSubchannel(); - - assertEquals(sc2, loadBalancer.getStickinessMapForTest().get("my-sticky-value").value); - - // shutdown channel via name resolver change - List newServers = new ArrayList<>(servers); - newServers.remove(sc2.getAddresses()); - - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(newServers).setAttributes(attributes).build()); - - verify(sc2, times(1)).shutdown(); - - deliverSubchannelState(sc2, ConnectivityStateInfo.forNonError(SHUTDOWN)); - - assertNull(loadBalancer.getStickinessMapForTest().get("my-sticky-value").value); - - assertEquals(nextSubchannel(sc2, allSubchannels), - picker.pickSubchannel(mockArgs).getSubchannel()); - assertThat(loadBalancer.getStickinessMapForTest()).hasSize(1); - verify(mockArgs, atLeast(2)).getHeaders(); - } - - @Test - @SuppressWarnings("deprecation") // migrate to parsed object - public void stickiness_resolveTwice_metadataKeyChanged() { - Map serviceConfig1 = new HashMap<>(); - serviceConfig1.put("stickinessMetadataKey", "my-sticky-key1"); - Attributes attributes1 = Attributes.newBuilder() - .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig1).build(); - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes1).build()); - Map stickinessMap1 = loadBalancer.getStickinessMapForTest(); - - Map serviceConfig2 = new HashMap<>(); - serviceConfig2.put("stickinessMetadataKey", "my-sticky-key2"); - Attributes attributes2 = Attributes.newBuilder() - .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig2).build(); - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes2).build()); - Map stickinessMap2 = loadBalancer.getStickinessMapForTest(); - - assertNotSame(stickinessMap1, stickinessMap2); - } - - @Test - @SuppressWarnings("deprecation") // migrate to parsed object - public void stickiness_resolveTwice_metadataKeyUnChanged() { - Map serviceConfig1 = new HashMap<>(); - serviceConfig1.put("stickinessMetadataKey", "my-sticky-key1"); - Attributes attributes1 = Attributes.newBuilder() - .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig1).build(); - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes1).build()); - Map stickinessMap1 = loadBalancer.getStickinessMapForTest(); - - loadBalancer.handleResolvedAddresses( - ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(attributes1).build()); - Map stickinessMap2 = loadBalancer.getStickinessMapForTest(); - - assertSame(stickinessMap1, stickinessMap2); - } - @Test(expected = IllegalArgumentException.class) public void readyPicker_emptyList() { // ready picker list must be non-empty - new ReadyPicker(Collections.emptyList(), 0, null); + new ReadyPicker(Collections.emptyList(), 0); } @Test @@ -782,24 +418,22 @@ public void internalPickerComparisons() { Iterator subchannelIterator = subchannels.values().iterator(); Subchannel sc1 = subchannelIterator.next(); Subchannel sc2 = subchannelIterator.next(); - StickinessState stickinessState = new StickinessState("stick-key"); - ReadyPicker ready1 = new ReadyPicker(Arrays.asList(sc1, sc2), 0, null); - ReadyPicker ready2 = new ReadyPicker(Arrays.asList(sc1), 0, null); - ReadyPicker ready3 = new ReadyPicker(Arrays.asList(sc2, sc1), 1, null); - ReadyPicker ready4 = new ReadyPicker(Arrays.asList(sc1, sc2), 1, stickinessState); - ReadyPicker ready5 = new ReadyPicker(Arrays.asList(sc2, sc1), 0, stickinessState); + ReadyPicker ready1 = new ReadyPicker(Arrays.asList(sc1, sc2), 0); + ReadyPicker ready2 = new ReadyPicker(Arrays.asList(sc1), 0); + ReadyPicker ready3 = new ReadyPicker(Arrays.asList(sc2, sc1), 1); + ReadyPicker ready4 = new ReadyPicker(Arrays.asList(sc1, sc2), 1); + ReadyPicker ready5 = new ReadyPicker(Arrays.asList(sc2, sc1), 0); assertTrue(emptyOk1.isEquivalentTo(emptyOk2)); assertFalse(emptyOk1.isEquivalentTo(emptyErr)); assertFalse(ready1.isEquivalentTo(ready2)); assertTrue(ready1.isEquivalentTo(ready3)); - assertFalse(ready3.isEquivalentTo(ready4)); + assertTrue(ready3.isEquivalentTo(ready4)); assertTrue(ready4.isEquivalentTo(ready5)); assertFalse(emptyOk1.isEquivalentTo(ready1)); assertFalse(ready1.isEquivalentTo(emptyOk1)); } - private static List getList(SubchannelPicker picker) { return picker instanceof ReadyPicker ? ((ReadyPicker) picker).getList() : Collections.emptyList();