grpc
/
grpc-java
mirror of https://github.com/grpc/grpc-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Change HappyEyeballs and new pick first LB flags default value to false (#11120) 

* Change HappyEyeballs flag default value to false since some G3 users are seeing problems.
Put the flag logic in a common place for PickFirstLeafLoadBalancer & WRR's test.

* Set expected requestConnection count based on whether happy eyeballs is enabled or not

* Disable new PickFirstLB

* Fix test expectations to handle both new and old PF LB paths.
This commit is contained in:
Larry Safran 2024-05-08 10:08:23 -07:00 committed by GitHub
parent d366d74fa6
commit 59b189bf91
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 47 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java
View File

@ -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<SocketAddress, SubchannelData> 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");

13
core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java
View File

@ -33,10 +33,21 @@ import java.util.Map;
* 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() {

8
core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java
View File

@ -142,8 +142,8 @@ public class PickFirstLeafLoadBalancerTest {
@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 class PickFirstLeafLoadBalancerTest {
@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);
}

27
rls/src/test/java/io/grpc/rls/RlsLoadBalancerTest.java
View File

@ -70,6 +70,7 @@ import io.grpc.inprocess.InProcessChannelBuilder;
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.internal.testing.StreamRecorder;
import io.grpc.lookup.v1.RouteLookupServiceGrpc;
@ -212,7 +213,8 @@ public class RlsLoadBalancerTest {
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");
@ -244,6 +246,8 @@ public class RlsLoadBalancerTest {
.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();
@ -258,7 +262,8 @@ public class RlsLoadBalancerTest {
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);
@ -367,18 +372,22 @@ public class RlsLoadBalancerTest {
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;
@ -389,7 +398,8 @@ public class RlsLoadBalancerTest {
(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);
@ -433,6 +443,8 @@ public class RlsLoadBalancerTest {
.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();
@ -469,7 +481,8 @@ public class RlsLoadBalancerTest {
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();

2
xds/src/main/java/io/grpc/xds/XdsEndpointResource.java
View File

@ -101,7 +101,7 @@ class XdsEndpointResource extends XdsResourceType<EdsUpdate> {
}
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)

10
xds/src/test/java/io/grpc/xds/RingHashLoadBalancerTest.java
View File

@ -151,7 +151,8 @@ public class RingHashLoadBalancerTest {
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 class RingHashLoadBalancerTest {
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 class RingHashLoadBalancerTest {
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);

4
xds/src/test/java/io/grpc/xds/WeightedRoundRobinLoadBalancerTest.java
View File

@ -64,7 +64,7 @@ import io.grpc.SynchronizationContext;
import io.grpc.inprocess.InProcessChannelBuilder;
import io.grpc.inprocess.InProcessServerBuilder;
import io.grpc.internal.FakeClock;
import io.grpc.internal.GrpcUtil;
import io.grpc.internal.PickFirstLoadBalancerProvider;
import io.grpc.internal.TestUtils;
import io.grpc.internal.testing.StreamRecorder;
import io.grpc.services.InternalCallMetricRecorder;
@ -595,7 +595,7 @@ public class WeightedRoundRobinLoadBalancerTest {
}
private boolean isEnabledHappyEyeballs() {
return GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_DUALSTACK_ENDPOINTS", true);
return PickFirstLoadBalancerProvider.isEnabledHappyEyeballs();
}
@Test