Merge pull request #2845 from murgatroid99/grpc-js-xds_config_tears_f…

…ixes grpc-js-xds: Fix a couple of bugs with the config tears change
grpc · Nov 7, 2024 · 83b6e60 · 83b6e60
2 parents 8a31431 + eee7030
commit 83b6e60
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 4 deletions.
diff --git a/packages/grpc-js-xds/src/xds-dependency-manager.ts b/packages/grpc-js-xds/src/xds-dependency-manager.ts
@@ -653,8 +653,9 @@ export class XdsDependencyManager {
           this.subscribedClusters[clusterName] -= 1;
           if (this.subscribedClusters[clusterName] <= 0) {
             delete this.subscribedClusters[clusterName];
-            this.pruneOrphanClusters();
-            this.maybeSendUpdate();
+            if (this.pruneOrphanClusters()) {
+              this.maybeSendUpdate();
+            }
           }
         }
       }
@@ -682,7 +683,12 @@ export class XdsDependencyManager {
     delete this.clusterForest[clusterName];
   }
 
-  private pruneOrphanClusters() {
+  /**
+   * Prune any clusters that are not descendents of any root clusters,
+   * including subscribed clusters.
+   * @returns True if any clusters were pruned, false otherwise
+   */
+  private pruneOrphanClusters(): boolean {
     const toCheck = [...this.clusterRoots, ...Object.keys(this.subscribedClusters)];
     const visited = new Set<string>();
     while(toCheck.length > 0) {
@@ -695,11 +701,14 @@ export class XdsDependencyManager {
       }
       visited.add(next);
     }
+    let removedAnyClusters = false;
     for (const clusterName of Object.keys(this.clusterForest)) {
       if (!visited.has(clusterName)) {
+        removedAnyClusters = true;
         this.removeCluster(clusterName);
       }
     }
+    return removedAnyClusters;
   }
 
   private handleRouteConfig(routeConfig: RouteConfiguration__Output) {

diff --git a/packages/grpc-js/src/backoff-timeout.ts b/packages/grpc-js/src/backoff-timeout.ts
@@ -107,7 +107,7 @@ export class BackoffTimeout {
   private runTimer(delay: number) {
     this.endTime = this.startTime;
     this.endTime.setMilliseconds(
-      this.endTime.getMilliseconds() + this.nextDelay
+      this.endTime.getMilliseconds() + delay
     );
     clearTimeout(this.timerId);
     this.timerId = setTimeout(() => {

diff --git a/packages/grpc-js/src/load-balancer-pick-first.ts b/packages/grpc-js/src/load-balancer-pick-first.ts
@@ -275,6 +275,13 @@ export class PickFirstLoadBalancer implements LoadBalancer {
           new PickFirstPicker(this.currentPick)
         );
       }
+    } else if (this.latestAddressList?.length === 0) {
+      this.updateState(
+        ConnectivityState.TRANSIENT_FAILURE,
+        new UnavailablePicker({
+          details: `No connection established. Last error: ${this.lastError}`,
+        })
+      );
     } else if (this.children.length === 0) {
       this.updateState(ConnectivityState.IDLE, new QueuePicker(this));
     } else {

diff --git a/packages/grpc-js/test/test-pick-first.ts b/packages/grpc-js/test/test-pick-first.ts
@@ -690,6 +690,19 @@ describe('pick_first load balancing policy', () => {
       });
     });
   });
+  it('Should report TRANSIENT_FAILURE with no addresses', done => {
+    const channelControlHelper = createChildChannelControlHelper(
+      baseChannelControlHelper,
+      {
+        updateState: updateStateCallBackForExpectedStateSequence(
+          [ConnectivityState.TRANSIENT_FAILURE],
+          done
+        ),
+      }
+    );
+    const pickFirst = new PickFirstLoadBalancer(channelControlHelper, creds, {});
+    pickFirst.updateAddressList([], config);
+  });
   describe('Address list randomization', () => {
     const shuffleConfig = new PickFirstLoadBalancingConfig(true);
     it('Should pick different subchannels after multiple updates', done => {