From f20167d60247588fa83a82620333c2e1e3216644 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 25 Jul 2024 13:28:49 -0700 Subject: [PATCH] util: Replace RR.EmptyPicker with FixedResultPicker --- .../io/grpc/util/RoundRobinLoadBalancer.java | 22 ++---------------- .../grpc/util/RoundRobinLoadBalancerTest.java | 23 +++++++++++-------- 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java b/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java index a8d829cfb8..0804978cf7 100644 --- a/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java +++ b/util/src/main/java/io/grpc/util/RoundRobinLoadBalancer.java @@ -42,7 +42,7 @@ import java.util.concurrent.atomic.AtomicInteger; */ final class RoundRobinLoadBalancer extends MultiChildLoadBalancer { private final AtomicInteger sequence = new AtomicInteger(new Random().nextInt()); - private SubchannelPicker currentPicker = new EmptyPicker(); + private SubchannelPicker currentPicker = new FixedResultPicker(PickResult.withNoResult()); public RoundRobinLoadBalancer(Helper helper) { super(helper); @@ -68,7 +68,7 @@ final class RoundRobinLoadBalancer extends MultiChildLoadBalancer { } if (isConnecting) { - updateBalancingState(CONNECTING, new EmptyPicker()); + updateBalancingState(CONNECTING, new FixedResultPicker(PickResult.withNoResult())); } else { updateBalancingState(TRANSIENT_FAILURE, createReadyPicker(getChildLbStates())); } @@ -180,22 +180,4 @@ final class RoundRobinLoadBalancer extends MultiChildLoadBalancer { && new HashSet<>(subchannelPickers).containsAll(other.subchannelPickers); } } - - @VisibleForTesting - static final class EmptyPicker extends SubchannelPicker { - @Override - public PickResult pickSubchannel(PickSubchannelArgs args) { - return PickResult.withNoResult(); - } - - @Override - public int hashCode() { - return getClass().hashCode(); - } - - @Override - public boolean equals(Object o) { - return o instanceof EmptyPicker; - } - } } diff --git a/util/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java b/util/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java index 449230f0f4..743bbbef79 100644 --- a/util/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java +++ b/util/src/test/java/io/grpc/util/RoundRobinLoadBalancerTest.java @@ -46,7 +46,9 @@ import io.grpc.ConnectivityStateInfo; import io.grpc.EquivalentAddressGroup; import io.grpc.LoadBalancer; import io.grpc.LoadBalancer.CreateSubchannelArgs; +import io.grpc.LoadBalancer.FixedResultPicker; import io.grpc.LoadBalancer.Helper; +import io.grpc.LoadBalancer.PickResult; import io.grpc.LoadBalancer.PickSubchannelArgs; import io.grpc.LoadBalancer.ResolvedAddresses; import io.grpc.LoadBalancer.Subchannel; @@ -54,7 +56,6 @@ import io.grpc.LoadBalancer.SubchannelPicker; import io.grpc.Status; import io.grpc.internal.TestUtils; import io.grpc.util.MultiChildLoadBalancer.ChildLbState; -import io.grpc.util.RoundRobinLoadBalancer.EmptyPicker; import io.grpc.util.RoundRobinLoadBalancer.ReadyPicker; import java.net.SocketAddress; import java.util.ArrayList; @@ -84,6 +85,8 @@ import org.mockito.junit.MockitoRule; @RunWith(JUnit4.class) public class RoundRobinLoadBalancerTest { private static final Attributes.Key MAJOR_KEY = Attributes.Key.create("major-key"); + private static final SubchannelPicker EMPTY_PICKER = + new FixedResultPicker(PickResult.withNoResult()); @Rule public final MockitoRule mocks = MockitoJUnit.rule(); @@ -248,7 +251,7 @@ public class RoundRobinLoadBalancerTest { ChildLbState childLbState = loadBalancer.getChildLbStates().iterator().next(); Subchannel subchannel = subchannels.get(Arrays.asList(childLbState.getEag())); - inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class)); + inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER)); assertThat(childLbState.getCurrentState()).isEqualTo(CONNECTING); deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(READY)); @@ -261,7 +264,7 @@ public class RoundRobinLoadBalancerTest { ConnectivityStateInfo.forTransientFailure(error)); assertThat(childLbState.getCurrentState()).isEqualTo(TRANSIENT_FAILURE); AbstractTestHelper.refreshInvokedAndUpdateBS(inOrder, CONNECTING, mockHelper, pickerCaptor); - assertThat(pickerCaptor.getValue()).isInstanceOf(EmptyPicker.class); + assertThat(pickerCaptor.getValue()).isEqualTo(EMPTY_PICKER); deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(IDLE)); inOrder.verify(mockHelper).refreshNameResolution(); @@ -277,7 +280,7 @@ public class RoundRobinLoadBalancerTest { InOrder inOrder = inOrder(mockHelper); Status addressesAcceptanceStatus = acceptAddresses(servers, Attributes.EMPTY); assertThat(addressesAcceptanceStatus.isOk()).isTrue(); - inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class)); + inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER)); loadBalancer.shutdown(); for (ChildLbState child : loadBalancer.getChildLbStates()) { @@ -297,7 +300,7 @@ public class RoundRobinLoadBalancerTest { Status addressesAcceptanceStatus = acceptAddresses(servers, Attributes.EMPTY); assertThat(addressesAcceptanceStatus.isOk()).isTrue(); - inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class)); + inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER)); Map childToSubChannelMap = new HashMap<>(); // Simulate state transitions for each subchannel individually. @@ -336,7 +339,7 @@ public class RoundRobinLoadBalancerTest { assertThat(addressesAcceptanceStatus.isOk()).isTrue(); verify(mockHelper, times(3)).createSubchannel(any(CreateSubchannelArgs.class)); - inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class)); + inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER)); // Simulate state transitions for each subchannel individually. for (ChildLbState child : loadBalancer.getChildLbStates()) { @@ -352,7 +355,7 @@ public class RoundRobinLoadBalancerTest { deliverSubchannelState(sc, ConnectivityStateInfo.forNonError(IDLE)); inOrder.verify(mockHelper).refreshNameResolution(); verify(sc, times(2)).requestConnection(); - inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), isA(EmptyPicker.class)); + inOrder.verify(mockHelper).updateBalancingState(eq(CONNECTING), eq(EMPTY_PICKER)); } AbstractTestHelper.verifyNoMoreMeaningfulInteractions(mockHelper); @@ -461,7 +464,7 @@ public class RoundRobinLoadBalancerTest { Iterator pickers = pickerCaptor.getAllValues().iterator(); // The picker is incrementally updated as subchannels become READY assertEquals(CONNECTING, stateIterator.next()); - assertThat(pickers.next()).isInstanceOf(EmptyPicker.class); + assertThat(pickers.next()).isEqualTo(EMPTY_PICKER); assertEquals(READY, stateIterator.next()); assertThat(getList(pickers.next())).containsExactly(sc1); assertEquals(READY, stateIterator.next()); @@ -492,8 +495,8 @@ public class RoundRobinLoadBalancerTest { @Test public void internalPickerComparisons() { - SubchannelPicker empty1 = new EmptyPicker(); - SubchannelPicker empty2 = new EmptyPicker(); + SubchannelPicker empty1 = new FixedResultPicker(PickResult.withNoResult()); + SubchannelPicker empty2 = new FixedResultPicker(PickResult.withNoResult()); AtomicInteger seq = new AtomicInteger(0); acceptAddresses(servers, Attributes.EMPTY); // create subchannels