diff --git a/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java b/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java index bcab9991f7f..3846656355c 100644 --- a/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java +++ b/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java @@ -59,8 +59,6 @@ final class PickFirstLeafLoadBalancer extends LoadBalancer { private static final Logger log = Logger.getLogger(PickFirstLeafLoadBalancer.class.getName()); @VisibleForTesting static final int CONNECTION_DELAY_INTERVAL_MS = 250; - public static final String GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS = - "GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS"; private final Helper helper; private final Map subchannels = new HashMap<>(); private Index addressIndex; @@ -71,7 +69,7 @@ final class PickFirstLeafLoadBalancer extends LoadBalancer { private ConnectivityState rawConnectivityState = IDLE; private ConnectivityState concludedState = IDLE; private final boolean enableHappyEyeballs = - GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, true); + PickFirstLoadBalancerProvider.isEnabledHappyEyeballs(); PickFirstLeafLoadBalancer(Helper helper) { this.helper = checkNotNull(helper, "helper"); diff --git a/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java b/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java index dd862fe704b..ad33a0b8411 100644 --- a/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java +++ b/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java @@ -33,10 +33,21 @@ * down the address list and sticks to the first that works. */ public final class PickFirstLoadBalancerProvider extends LoadBalancerProvider { + public static final String GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS = + "GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS"; private static final String SHUFFLE_ADDRESS_LIST_KEY = "shuffleAddressList"; static boolean enableNewPickFirst = - GrpcUtil.getFlag("GRPC_EXPERIMENTAL_ENABLE_NEW_PICK_FIRST", true); + GrpcUtil.getFlag("GRPC_EXPERIMENTAL_ENABLE_NEW_PICK_FIRST", false); + + public static boolean isEnabledHappyEyeballs() { + return GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, false); + } + + @VisibleForTesting + public static boolean isEnableNewPickFirst() { + return enableNewPickFirst; + } @Override public boolean isAvailable() { diff --git a/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java b/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java index 92222ac9af6..eceb9500ef0 100644 --- a/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java +++ b/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java @@ -142,8 +142,8 @@ public void uncaughtException(Thread t, Throwable e) { @Before public void setUp() { originalHappyEyeballsEnabledValue = - System.getProperty(PickFirstLeafLoadBalancer.GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS); - System.setProperty(PickFirstLeafLoadBalancer.GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, + System.getProperty(PickFirstLoadBalancerProvider.GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS); + System.setProperty(PickFirstLoadBalancerProvider.GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, enableHappyEyeballs ? "true" : "false"); for (int i = 1; i <= 5; i++) { @@ -173,9 +173,9 @@ public void setUp() { @After public void tearDown() { if (originalHappyEyeballsEnabledValue == null) { - System.clearProperty(PickFirstLeafLoadBalancer.GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS); + System.clearProperty(PickFirstLoadBalancerProvider.GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS); } else { - System.setProperty(PickFirstLeafLoadBalancer.GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, + System.setProperty(PickFirstLoadBalancerProvider.GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, originalHappyEyeballsEnabledValue); } diff --git a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java index fd29c871ada..6b4ac272322 100644 --- a/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java +++ b/rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java @@ -66,6 +66,7 @@ import io.grpc.inprocess.InProcessServerBuilder; import io.grpc.internal.FakeClock; import io.grpc.internal.JsonParser; +import io.grpc.internal.PickFirstLoadBalancerProvider; import io.grpc.internal.PickSubchannelArgsImpl; import io.grpc.lookup.v1.RouteLookupServiceGrpc; import io.grpc.rls.RlsLoadBalancer.CachingRlsLbClientBuilderProvider; @@ -206,7 +207,8 @@ public void lb_serverStatusCodeConversion() throws Exception { subchannel.updateState(ConnectivityStateInfo.forNonError(ConnectivityState.READY)); res = picker.pickSubchannel(fakeSearchMethodArgs); assertThat(res.getStatus().getCode()).isEqualTo(Status.Code.OK); - verifyLongCounterAdd("grpc.lb.rls.target_picks", 1, 1, "wilderness", "complete"); + int expectedTimes = PickFirstLoadBalancerProvider.isEnabledNewPickFirst() ? 1 : 2; + verifyLongCounterAdd("grpc.lb.rls.target_picks", expectedTimes, 1, "wilderness", "complete"); // Check on conversion Throwable cause = new Throwable("cause"); @@ -238,6 +240,8 @@ public void lb_working_withDefaultTarget_rlsResponding() throws Exception { .updateBalancingState(eq(ConnectivityState.CONNECTING), any(SubchannelPicker.class)); inOrder.verify(helper, atLeast(0)).getSynchronizationContext(); inOrder.verify(helper, atLeast(0)).getScheduledExecutorService(); + inOrder.verify(helper, atLeast(0)).getMetricRecorder(); + inOrder.verify(helper, atLeast(0)).getChannelTarget(); inOrder.verifyNoMoreInteractions(); assertThat(res.getStatus().isOk()).isTrue(); @@ -252,7 +256,8 @@ public void lb_working_withDefaultTarget_rlsResponding() throws Exception { res = picker.pickSubchannel(searchSubchannelArgs); assertThat(subchannelIsReady(res.getSubchannel())).isTrue(); assertThat(res.getSubchannel()).isSameInstanceAs(searchSubchannel); - verifyLongCounterAdd("grpc.lb.rls.target_picks", 1, 1, "wilderness", "complete"); + int expectedTimes = PickFirstLoadBalancerProvider.isEnabledNewPickFirst() ? 1 : 2; + verifyLongCounterAdd("grpc.lb.rls.target_picks", expectedTimes, 1, "wilderness", "complete"); // rescue should be pending status although the overall channel state is READY res = picker.pickSubchannel(rescueSubchannelArgs); @@ -323,18 +328,22 @@ public void lb_working_withDefaultTarget_noRlsResponse() throws Exception { inOrder.verify(helper).getMetricRecorder(); inOrder.verify(helper).getChannelTarget(); inOrder.verifyNoMoreInteractions(); - verifyLongCounterAdd("grpc.lb.rls.default_target_picks", 1, 1, "defaultTarget", "complete"); + int times = PickFirstLoadBalancerProvider.isEnabledNewPickFirst() ? 1 : 2; + verifyLongCounterAdd("grpc.lb.rls.default_target_picks", times, 1, + "defaultTarget", "complete"); verifyNoMoreInteractions(mockMetricRecorder); Subchannel subchannel = picker.pickSubchannel(searchSubchannelArgs).getSubchannel(); assertThat(subchannelIsReady(subchannel)).isTrue(); assertThat(subchannel).isSameInstanceAs(fallbackSubchannel); - verifyLongCounterAdd("grpc.lb.rls.default_target_picks", 2, 1, "defaultTarget", "complete"); + verifyLongCounterAdd("grpc.lb.rls.default_target_picks", ++times, 1, "defaultTarget", + "complete"); subchannel = picker.pickSubchannel(searchSubchannelArgs).getSubchannel(); assertThat(subchannelIsReady(subchannel)).isTrue(); assertThat(subchannel).isSameInstanceAs(fallbackSubchannel); - verifyLongCounterAdd("grpc.lb.rls.default_target_picks", 3, 1, "defaultTarget", "complete"); + verifyLongCounterAdd("grpc.lb.rls.default_target_picks", ++times, 1, "defaultTarget", + "complete"); // Make sure that when RLS starts communicating that default stops being used fakeThrottler.nextResult = false; @@ -345,7 +354,8 @@ public void lb_working_withDefaultTarget_noRlsResponse() throws Exception { (FakeSubchannel) markReadyAndGetPickResult(inOrder, searchSubchannelArgs).getSubchannel(); assertThat(searchSubchannel).isNotNull(); assertThat(searchSubchannel).isNotSameInstanceAs(fallbackSubchannel); - verifyLongCounterAdd("grpc.lb.rls.target_picks", 1, 1, "wilderness", "complete"); + times = PickFirstLoadBalancerProvider.isEnabledNewPickFirst() ? 1 : 2; + verifyLongCounterAdd("grpc.lb.rls.target_picks", times, 1, "wilderness", "complete"); // create rescue subchannel picker.pickSubchannel(rescueSubchannelArgs); @@ -389,6 +399,8 @@ public void lb_working_withoutDefaultTarget() throws Exception { .updateBalancingState(eq(ConnectivityState.CONNECTING), any(SubchannelPicker.class)); inOrder.verify(helper, atLeast(0)).getSynchronizationContext(); inOrder.verify(helper, atLeast(0)).getScheduledExecutorService(); + inOrder.verify(helper, atLeast(0)).getMetricRecorder(); + inOrder.verify(helper, atLeast(0)).getChannelTarget(); inOrder.verifyNoMoreInteractions(); assertThat(res.getStatus().isOk()).isTrue(); @@ -425,7 +437,8 @@ public void lb_working_withoutDefaultTarget() throws Exception { res = picker.pickSubchannel(newPickSubchannelArgs(fakeSearchMethod)); assertThat(res.getStatus().isOk()).isFalse(); assertThat(subchannelIsReady(res.getSubchannel())).isFalse(); - verifyLongCounterAdd("grpc.lb.rls.target_picks", 1, 1, "wilderness", "complete"); + int expectedTimes = PickFirstLoadBalancerProvider.isEnabledNewPickFirst() ? 1 : 2; + verifyLongCounterAdd("grpc.lb.rls.target_picks", expectedTimes, 1, "wilderness", "complete"); res = picker.pickSubchannel(newPickSubchannelArgs(fakeRescueMethod)); assertThat(subchannelIsReady(res.getSubchannel())).isTrue(); diff --git a/xds/src/main/java/io/grpc/xds/XdsEndpointResource.java b/xds/src/main/java/io/grpc/xds/XdsEndpointResource.java index 770217bfbaa..0c7e8f46bcc 100644 --- a/xds/src/main/java/io/grpc/xds/XdsEndpointResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsEndpointResource.java @@ -101,7 +101,7 @@ protected EdsUpdate doParse(Args args, Message unpackedMessage) throws ResourceI } private static boolean isEnabledXdsDualStack() { - return GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, true); + return GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS, false); } private static EdsUpdate processClusterLoadAssignment(ClusterLoadAssignment assignment) diff --git a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java index 339779cf5e0..de871cdd8f1 100644 --- a/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java @@ -151,7 +151,8 @@ public void subchannelLazyConnectUntilPicked() { assertThat(result.getStatus().isOk()).isTrue(); assertThat(result.getSubchannel()).isNull(); Subchannel subchannel = Iterables.getOnlyElement(subchannels.values()); - verify(subchannel).requestConnection(); + int expectedTimes = PickFirstLoadBalancerProvider.isEnabledHappyEyeballs() ? 1 : 2; + verify(subchannel, times(expectedTimes)).requestConnection(); verify(helper).updateBalancingState(eq(CONNECTING), any(SubchannelPicker.class)); verify(helper).createSubchannel(any(CreateSubchannelArgs.class)); deliverSubchannelState(subchannel, CSI_CONNECTING); @@ -185,7 +186,8 @@ public void subchannelNotAutoReconnectAfterReenteringIdle() { pickerCaptor.getValue().pickSubchannel(args); Subchannel subchannel = subchannels.get(Collections.singletonList(childLbState.getEag())); InOrder inOrder = Mockito.inOrder(helper, subchannel); - inOrder.verify(subchannel).requestConnection(); + int expectedTimes = PickFirstLoadBalancerProvider.isEnabledHappyEyeballs() ? 1 : 2; + inOrder.verify(subchannel, times(expectedTimes)).requestConnection(); deliverSubchannelState(subchannel, CSI_READY); inOrder.verify(helper).updateBalancingState(eq(READY), any(SubchannelPicker.class)); deliverSubchannelState(subchannel, ConnectivityStateInfo.forNonError(IDLE)); @@ -443,7 +445,9 @@ public void skipFailingHosts_pickNextNonFailingHost() { PickResult result = pickerCaptor.getValue().pickSubchannel(args); assertThat(result.getStatus().isOk()).isTrue(); assertThat(result.getSubchannel()).isNull(); // buffer request - verify(getSubChannel(servers.get(1))).requestConnection(); // kicked off connection to server2 + int expectedTimes = PickFirstLoadBalancerProvider.isEnabledHappyEyeballs() ? 1 : 2; + // verify kicked off connection to server2 + verify(getSubChannel(servers.get(1)), times(expectedTimes)).requestConnection(); assertThat(subchannels.size()).isEqualTo(2); // no excessive connection deliverSubchannelState(getSubChannel(servers.get(1)), CSI_CONNECTING); diff --git a/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java index 6ef69c5ebdb..f71400ba46d 100644 --- a/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java @@ -56,7 +56,7 @@ import io.grpc.Status; import io.grpc.SynchronizationContext; import io.grpc.internal.FakeClock; -import io.grpc.internal.GrpcUtil; +import io.grpc.internal.PickFirstLoadBalancerProvider; import io.grpc.internal.TestUtils; import io.grpc.services.InternalCallMetricRecorder; import io.grpc.services.MetricReport; @@ -579,7 +579,7 @@ weightedChild2.new OrcaReportListener(weightedConfig.errorUtilizationPenalty).on } private boolean isEnabledHappyEyeballs() { - return GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS", true); + return PickFirstLoadBalancerProvider.isEnabledHappyEyeballs(); } @Test