grpc-js: Some fixes for how idleness and reresolution are handled

packages/grpc-js/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@grpc/grpc-js",
-  "version": "0.6.8",
+  "version": "0.6.9",
   "description": "gRPC Library for Node - pure JS implementation",
   "homepage": "https://grpc.io/",
   "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",

packages/grpc-js/src/load-balancer-pick-first.ts

@@ -152,6 +152,12 @@ export class PickFirstLoadBalancer implements LoadBalancer {
         this.pickSubchannel(subchannel);
         return;
       } else {
+        if (this.triedAllSubchannels && this.subchannelStateCounts[ConnectivityState.IDLE] === this.subchannels.length) {
+          /* If all of the subchannels are IDLE we should go back to a
+           * basic IDLE state where there is no subchannel list to avoid
+           * holding unused resources */
+          this.resetSubchannelList();
+        }
         if (this.currentPick === null) {
           if (this.triedAllSubchannels) {
             let newLBState: ConnectivityState;
@@ -190,6 +196,7 @@ export class PickFirstLoadBalancer implements LoadBalancer {
           this.pickedSubchannelStateListener
         );
         if (this.subchannels.length > 0) {
+          if (this.triedAllSubchannels) {
             let newLBState: ConnectivityState;
             if (this.subchannelStateCounts[ConnectivityState.CONNECTING] > 0) {
               newLBState = ConnectivityState.CONNECTING;
@@ -203,6 +210,12 @@ export class PickFirstLoadBalancer implements LoadBalancer {
             } else {
               this.updateState(newLBState, new QueuePicker(this));
             }
+          } else {
+            this.updateState(
+              ConnectivityState.CONNECTING,
+              new QueuePicker(this)
+            );
+          }
         } else {
           /* We don't need to backoff here because this only happens if a
            * subchannel successfully connects then disconnects, so it will not

packages/grpc-js/src/resolving-load-balancer.ts

@@ -77,6 +77,8 @@ export class ResolvingLoadBalancer implements LoadBalancer {
    */
   private innerBalancerState: ConnectivityState = ConnectivityState.IDLE;
 
+  private innerBalancerPicker: Picker = new UnavailablePicker();
+
   /**
    * The most recent reported state of the pendingReplacementLoadBalancer.
    * Starts at IDLE for type simplicity. This should get updated as soon as the
@@ -104,6 +106,12 @@ export class ResolvingLoadBalancer implements LoadBalancer {
    */
   private readonly backoffTimeout: BackoffTimeout;
 
+  /**
+   * Indicates whether we should attempt to resolve again after the backoff
+   * timer runs out.
+   */
+  private continueResolving = false;
+
   /**
    * Wrapper class that behaves like a `LoadBalancer` and also handles name
    * resolution internally.
@@ -245,12 +253,22 @@ export class ResolvingLoadBalancer implements LoadBalancer {
       },
       updateState: (connectivityState: ConnectivityState, picker: Picker) => {
         this.innerBalancerState = connectivityState;
+        if (connectivityState === ConnectivityState.IDLE) {
+          picker = new QueuePicker(this);
+        }
+        this.innerBalancerPicker = picker;
         if (
           connectivityState !== ConnectivityState.READY &&
           this.pendingReplacementLoadBalancer !== null
         ) {
           this.switchOverReplacementBalancer();
         } else {
+          if (connectivityState === ConnectivityState.IDLE) {
+            if (this.innerLoadBalancer) {
+              this.innerLoadBalancer.destroy();
+              this.innerLoadBalancer = null;
+            }
+          }
           this.updateState(connectivityState, picker);
         }
       },
@@ -260,8 +278,10 @@ export class ResolvingLoadBalancer implements LoadBalancer {
            * making resolve requests, so we shouldn't make another one here.
            * In that case, the backoff timer callback will call
            * updateResolution */
-          if (!this.backoffTimeout.isRunning()) {
-            this.innerResolver.updateResolution();
+          if (this.backoffTimeout.isRunning()) {
+            this.continueResolving = true;
+          } else {
+            this.updateResolution();
           }
         }
       },
@@ -278,32 +298,54 @@ export class ResolvingLoadBalancer implements LoadBalancer {
         );
       },
       updateState: (connectivityState: ConnectivityState, picker: Picker) => {
+        if (connectivityState === ConnectivityState.IDLE) {
+          picker = new QueuePicker(this);
+        }
         this.replacementBalancerState = connectivityState;
         this.replacementBalancerPicker = picker;
         if (connectivityState === ConnectivityState.READY) {
           this.switchOverReplacementBalancer();
+        } else if (connectivityState === ConnectivityState.IDLE) {
+          if (this.pendingReplacementLoadBalancer) {
+            this.pendingReplacementLoadBalancer.destroy();
+            this.pendingReplacementLoadBalancer = null;
+          }
         }
       },
       requestReresolution: () => {
-        if (!this.backoffTimeout.isRunning()) {
         /* If the backoffTimeout is running, we're still backing off from
          * making resolve requests, so we shouldn't make another one here.
          * In that case, the backoff timer callback will call
          * updateResolution */
-          this.innerResolver.updateResolution();
+        if (this.backoffTimeout.isRunning()) {
+          this.continueResolving = true;
+        } else {
+          this.updateResolution();
         }
       },
     };
 
     this.backoffTimeout = new BackoffTimeout(() => {
+      if (this.continueResolving) {
+        this.updateResolution();
+        this.continueResolving = false;
+      } else {
         if (this.innerLoadBalancer === null) {
           this.updateState(ConnectivityState.IDLE, new QueuePicker(this));
         } else {
-        this.innerResolver.updateResolution();
+          this.updateState(this.innerBalancerState, this.innerBalancerPicker);
+        }
       }
     });
   }
 
+  private updateResolution() {
+    this.innerResolver.updateResolution();
+    if (this.innerLoadBalancer === null || this.innerBalancerState === ConnectivityState.IDLE) {
+      this.updateState(ConnectivityState.CONNECTING, new QueuePicker(this));
+    }
+  }
+
   private updateState(connectivitystate: ConnectivityState, picker: Picker) {
     trace(this.target + ' ' + ConnectivityState[this.currentState] + ' -> ' + ConnectivityState[connectivitystate]);
     this.currentState = connectivitystate;
@@ -323,6 +365,7 @@ export class ResolvingLoadBalancer implements LoadBalancer {
     );
     this.pendingReplacementLoadBalancer = null;
     this.innerBalancerState = this.replacementBalancerState;
+    this.innerBalancerPicker = this.replacementBalancerPicker;
     this.updateState(
       this.replacementBalancerState,
       this.replacementBalancerPicker
@@ -330,7 +373,7 @@ export class ResolvingLoadBalancer implements LoadBalancer {
   }
 
   private handleResolutionFailure(error: StatusObject) {
-    if (this.innerLoadBalancer === null) {
+    if (this.innerLoadBalancer === null || this.innerBalancerState === ConnectivityState.IDLE) {
       this.updateState(
         ConnectivityState.TRANSIENT_FAILURE,
         new UnavailablePicker(error)
@@ -344,7 +387,11 @@ export class ResolvingLoadBalancer implements LoadBalancer {
       this.innerLoadBalancer.exitIdle();
     }
     if (this.currentState === ConnectivityState.IDLE) {
-      this.innerResolver.updateResolution();
+      if (this.backoffTimeout.isRunning()) {
+        this.continueResolving = true;
+      } else {
+        this.updateResolution();
+      }
       this.updateState(
         ConnectivityState.CONNECTING,
         new QueuePicker(this)