Respect clientTtl

Author: JoshMock

lib/dispatcher/pool-base.js

@@ -43,7 +43,10 @@ class PoolBase extends DispatcherBase {
     if (this[kMaxIdle] < Infinity && this[kFree] > this[kMaxIdle]) {
       for (let i = 0; i < this[kClients].length; i++) {
         const client = this[kClients][i]
-        if (!client[kNeedDrain]) this[kRemoveClient](client)
+        if (!client[kNeedDrain]) {
+          if (client.ttl && client.ttl > Date.now()) continue
+          this[kRemoveClient](client)
+        }
         if (this[kFree] <= this[kMaxIdle]) break
       }
     }

lib/dispatcher/pool.js

@@ -82,7 +82,7 @@ class Pool extends PoolBase {
     this.on('connect', (origin, targets) => {
       if (clientTtl != null && clientTtl > 0) {
         for (const target of targets) {
-          Object.assign(target, { ttl: Date.now() })
+          Object.assign(target, { ttl: Date.now() + clientTtl })
         }
       }
     })
@@ -106,7 +106,7 @@ class Pool extends PoolBase {
     const clientTtlOption = this[kOptions].clientTtl
     for (const client of this[kClients]) {
       // check ttl of client and if it's stale, remove it from the pool
-      if (clientTtlOption != null && clientTtlOption > 0 && client.ttl && ((Date.now() - client.ttl) > clientTtlOption)) {
+      if (clientTtlOption != null && clientTtlOption > 0 && client.ttl && (client.ttl <= Date.now())) {
         this[kRemoveClient](client)
       } else if (!client[kNeedDrain]) {
         return client

lib/dispatcher/round-robin-pool.js

@@ -78,7 +78,7 @@ class RoundRobinPool extends PoolBase {
     this.on('connect', (origin, targets) => {
       if (clientTtl != null && clientTtl > 0) {
         for (const target of targets) {
-          Object.assign(target, { ttl: Date.now() })
+          Object.assign(target, { ttl: Date.now() + clientTtl })
         }
       }
     })
@@ -111,7 +111,7 @@ class RoundRobinPool extends PoolBase {
       const client = this[kClients][this[kIndex]]
 
       // Check if client is stale (TTL expired)
-      if (clientTtlOption != null && clientTtlOption > 0 && client.ttl && ((Date.now() - client.ttl) > clientTtlOption)) {
+      if (clientTtlOption != null && clientTtlOption > 0 && client.ttl && (client.ttl <= Date.now())) {
         this[kRemoveClient](client)
         checked++
         continue

test/node-test/agent.js

@@ -152,6 +152,37 @@ test('Pool enforces maxIdle', async (t) => {
   await p.completed
 })
 
+test('maxIdle respects clientTtl', async (t) => {
+  const p = tspl(t, { plan: 1 })
+
+  const handler = (_req, res) => res.end('ok')
+  const server = http.createServer({ joinDuplicateHeaders: true }, handler)
+  server.listen(0)
+  await once(server, 'listening')
+
+  const host = `http://localhost:${server.address().port}`
+  const dispatcher = new Pool(host, {
+    maxIdle: 3,
+    clientTtl: 2000,
+    keepAliveTimeout: 1000
+  })
+
+  t.after(closeClientAndServerAsPromise(dispatcher, server))
+
+  await Promise.all([
+    request(host, { dispatcher }),
+    request(host, { dispatcher }),
+    request(host, { dispatcher }),
+    request(host, { dispatcher }),
+    request(host, { dispatcher })
+  ])
+
+  await new Promise(resolve => setTimeout(resolve, 1))
+  p.equal(dispatcher.stats.free, 5)
+
+  await p.completed
+})
+
 test('agent should call callback after closing internal pools', async (t) => {
   const p = tspl(t, { plan: 2 })