From 82a8d573969690423d442fd9c85ebd6642a10d80 Mon Sep 17 00:00:00 2001
From: Eric Anderson <ejona@google.com>
Date: Sat, 17 Aug 2024 10:50:06 -0700
Subject: [PATCH] core: In PF, remove useless requestConnection for CONNECTING
 subchannel

It doesn't do anything.

Call scheduleNextConnection() unconditionally since it is responsible
for checking if `enableHappyEyeballs == true`. It's also surprising to
check in the CONNECTING case but not the IDLE case.
---
 .../grpc/internal/PickFirstLeafLoadBalancer.java  |  6 +-----
 .../internal/PickFirstLeafLoadBalancerTest.java   | 15 +++------------
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java b/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java
index e77afccfb6..815255ba6a 100644
--- a/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java
+++ b/core/src/main/java/io/grpc/internal/PickFirstLeafLoadBalancer.java
@@ -385,11 +385,7 @@ final class PickFirstLeafLoadBalancer extends LoadBalancer {
         scheduleNextConnection();
         break;
       case CONNECTING:
-        if (enableHappyEyeballs) {
-          scheduleNextConnection();
-        } else {
-          subchannelData.subchannel.requestConnection();
-        }
+        scheduleNextConnection();
         break;
       case TRANSIENT_FAILURE:
         addressIndex.increment();
diff --git a/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java b/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java
index 1ce024366c..a6e3f99ab3 100644
--- a/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java
+++ b/core/src/test/java/io/grpc/internal/PickFirstLeafLoadBalancerTest.java
@@ -361,11 +361,7 @@ public class PickFirstLeafLoadBalancerTest {
     // Second acceptResolvedAddresses shouldn't do anything
     loadBalancer.acceptResolvedAddresses(
         ResolvedAddresses.newBuilder().setAddresses(servers).setAttributes(affinity).build());
-    if (enableHappyEyeballs) {
-      inOrder.verify(mockSubchannel1, never()).requestConnection();
-    } else {
-      inOrder.verify(mockSubchannel1, times(1)).requestConnection();
-    }
+    inOrder.verify(mockSubchannel1, never()).requestConnection();
     inOrder.verify(mockHelper, never()).updateBalancingState(any(), any());
   }
 
@@ -862,8 +858,7 @@ public class PickFirstLeafLoadBalancerTest {
     loadBalancer.requestConnection();
     inOrder.verify(mockSubchannel2).start(stateListenerCaptor.capture());
     SubchannelStateListener stateListener2 = stateListenerCaptor.getValue();
-    int expectedRequests = enableHappyEyeballs ? 1 : 2;
-    inOrder.verify(mockSubchannel2, times(expectedRequests)).requestConnection();
+    inOrder.verify(mockSubchannel2).requestConnection();
 
     stateListener2.onSubchannelState(ConnectivityStateInfo.forNonError(CONNECTING));
 
@@ -871,11 +866,7 @@ public class PickFirstLeafLoadBalancerTest {
     loadBalancer.requestConnection();
     inOrder.verify(mockHelper, never()).updateBalancingState(any(), any());
     inOrder.verify(mockSubchannel1, never()).requestConnection();
-    if (enableHappyEyeballs) {
-      inOrder.verify(mockSubchannel2, never()).requestConnection();
-    } else {
-      inOrder.verify(mockSubchannel2).requestConnection();
-    }
+    inOrder.verify(mockSubchannel2, never()).requestConnection();
   }
 
   @Test