Merge pull request #2643 from murgatroid99/grpc-js_idle_timer_fix

grpc-js: Fix and optimize IDLE timeouts
grpc · Jan 16, 2024 · 5be83dd · 5be83dd
2 parents b8dc020 + 6da0b49
commit 5be83dd
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 10 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.13",
+  "version": "1.9.14",
   "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/internal-channel.ts b/packages/grpc-js/src/internal-channel.ts
@@ -184,6 +184,7 @@ export class InternalChannel {
   private callCount = 0;
   private idleTimer: NodeJS.Timeout | null = null;
   private readonly idleTimeoutMs: number;
+  private lastActivityTimestamp: Date;
 
   // Channelz info
   private readonly channelzEnabled: boolean = true;
@@ -409,6 +410,7 @@ export class InternalChannel {
         'Channel constructed \n' +
         error.stack?.substring(error.stack.indexOf('\n') + 1)
     );
+    this.lastActivityTimestamp = new Date();
   }
 
   private getChannelzInfo(): ChannelInfo {
@@ -556,19 +558,44 @@ export class InternalChannel {
     this.resolvingLoadBalancer.destroy();
     this.updateState(ConnectivityState.IDLE);
     this.currentPicker = new QueuePicker(this.resolvingLoadBalancer);
+    if (this.idleTimer) {
+      clearTimeout(this.idleTimer);
+      this.idleTimer = null;
+    }
   }
 
-  private maybeStartIdleTimer() {
-    if (this.connectivityState !== ConnectivityState.SHUTDOWN && this.callCount === 0) {
-      this.idleTimer = setTimeout(() => {
+  private startIdleTimeout(timeoutMs: number) {
+    this.idleTimer = setTimeout(() => {
+      if (this.callCount > 0) {
+        /* If there is currently a call, the channel will not go idle for a
+         * period of at least idleTimeoutMs, so check again after that time.
+         */
+        this.startIdleTimeout(this.idleTimeoutMs);
+        return;
+      }
+      const now = new Date();
+      const timeSinceLastActivity = now.valueOf() - this.lastActivityTimestamp.valueOf();
+      if (timeSinceLastActivity >= this.idleTimeoutMs) {
         this.trace(
           'Idle timer triggered after ' +
             this.idleTimeoutMs +
             'ms of inactivity'
         );
         this.enterIdle();
-      }, this.idleTimeoutMs);
-      this.idleTimer.unref?.();
+      } else {
+        /* Whenever the timer fires with the latest activity being too recent,
+         * set the timer again for the time when the time since the last
+         * activity is equal to the timeout. This should result in the timer
+         * firing no more than once every idleTimeoutMs/2 on average. */
+        this.startIdleTimeout(this.idleTimeoutMs - timeSinceLastActivity);
+      }
+    }, timeoutMs);
+    this.idleTimer.unref?.();
+  }
+
+  private maybeStartIdleTimer() {
+    if (this.connectivityState !== ConnectivityState.SHUTDOWN && !this.idleTimer) {
+      this.startIdleTimeout(this.idleTimeoutMs);
     }
   }
 
@@ -577,10 +604,6 @@ export class InternalChannel {
       this.callTracker.addCallStarted();
     }
     this.callCount += 1;
-    if (this.idleTimer) {
-      clearTimeout(this.idleTimer);
-      this.idleTimer = null;
-    }
   }
 
   private onCallEnd(status: StatusObject) {
@@ -592,6 +615,7 @@ export class InternalChannel {
       }
     }
     this.callCount -= 1;
+    this.lastActivityTimestamp = new Date();
     this.maybeStartIdleTimer();
   }
 
@@ -729,6 +753,7 @@ export class InternalChannel {
     const connectivityState = this.connectivityState;
     if (tryToConnect) {
       this.resolvingLoadBalancer.exitIdle();
+      this.lastActivityTimestamp = new Date();
       this.maybeStartIdleTimer();
     }
     return connectivityState;