Merge pull request #2619 from murgatroid99/grpc-js_idle_loop_fix

grpc-js: Make pick_first use exitIdle
grpc · Nov 16, 2023 · 85bc3c2 · 85bc3c2
2 parents 667bae6 + 8843706
commit 85bc3c2
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 17 deletions.
diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@grpc/grpc-js",
-  "version": "1.9.10",
+  "version": "1.9.11",
   "description": "gRPC Library for Node - pure JS implementation",
   "homepage": "https://grpc.io/",
   "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",

diff --git a/packages/grpc-js/src/load-balancer-pick-first.ts b/packages/grpc-js/src/load-balancer-pick-first.ts
@@ -186,6 +186,8 @@ export class PickFirstLoadBalancer implements LoadBalancer {
    */
   private lastError: string | null = null;
 
+  private latestAddressList: SubchannelAddress[] | null = null;
+
   /**
    * Load balancer that attempts to connect to each backend in the address list
    * in order, and picks the first one that connects, using it for every
@@ -404,19 +406,7 @@ export class PickFirstLoadBalancer implements LoadBalancer {
     this.requestedResolutionSinceLastUpdate = false;
   }
 
-  updateAddressList(
-    addressList: SubchannelAddress[],
-    lbConfig: LoadBalancingConfig
-  ): void {
-    if (!(lbConfig instanceof PickFirstLoadBalancingConfig)) {
-      return;
-    }
-    /* Previously, an update would be discarded if it was identical to the
-     * previous update, to minimize churn. Now the DNS resolver is
-     * rate-limited, so that is less of a concern. */
-    if (lbConfig.getShuffleAddressList()) {
-      addressList = shuffled(addressList);
-    }
+  private connectToAddressList(addressList: SubchannelAddress[]) {
     const newChildrenList = addressList.map(address => ({
       subchannel: this.channelControlHelper.createSubchannel(address, {}),
       hasReportedTransientFailure: false,
@@ -449,10 +439,27 @@ export class PickFirstLoadBalancer implements LoadBalancer {
     this.calculateAndReportNewState();
   }
 
+  updateAddressList(
+    addressList: SubchannelAddress[],
+    lbConfig: LoadBalancingConfig
+  ): void {
+    if (!(lbConfig instanceof PickFirstLoadBalancingConfig)) {
+      return;
+    }
+    /* Previously, an update would be discarded if it was identical to the
+     * previous update, to minimize churn. Now the DNS resolver is
+     * rate-limited, so that is less of a concern. */
+    if (lbConfig.getShuffleAddressList()) {
+      addressList = shuffled(addressList);
+    }
+    this.latestAddressList = addressList;
+    this.connectToAddressList(addressList);
+  }
+
   exitIdle() {
-    /* The pick_first LB policy is only in the IDLE state if it has no
-     * addresses to try to connect to and it has no picked subchannel.
-     * In that case, there is no meaningful action that can be taken here. */
+    if (this.currentState === ConnectivityState.IDLE && this.latestAddressList) {
+      this.connectToAddressList(this.latestAddressList);
+    }
   }
 
   resetBackoff() {

diff --git a/packages/grpc-js/test/test-pick-first.ts b/packages/grpc-js/test/test-pick-first.ts
@@ -550,6 +550,34 @@ describe('pick_first load balancing policy', () => {
       });
     });
   });
+  it('Should reconnect to the same address list if exitIdle is called', done => {
+    const currentStartState = ConnectivityState.READY;
+    const channelControlHelper = createChildChannelControlHelper(
+      baseChannelControlHelper,
+      {
+        createSubchannel: (subchannelAddress, subchannelArgs) => {
+          const subchannel = new MockSubchannel(
+            subchannelAddressToString(subchannelAddress),
+            currentStartState
+          );
+          subchannels.push(subchannel);
+          return subchannel;
+        },
+        updateState: updateStateCallBackForExpectedStateSequence(
+          [ConnectivityState.READY, ConnectivityState.IDLE, ConnectivityState.READY],
+          done
+        ),
+      }
+    );
+    const pickFirst = new PickFirstLoadBalancer(channelControlHelper);
+    pickFirst.updateAddressList([{ host: 'localhost', port: 1 }], config);
+    process.nextTick(() => {
+      subchannels[0].transitionToState(ConnectivityState.IDLE);
+      process.nextTick(() => {
+        pickFirst.exitIdle();
+      });
+    });
+  });
   describe('Address list randomization', () => {
     const shuffleConfig = new PickFirstLoadBalancingConfig(true);
     it('Should pick different subchannels after multiple updates', done => {