From e9466ddcd0957839bf5bfbbe3bfc2f7a399c07c2 Mon Sep 17 00:00:00 2001 From: Asadbek Raimov Date: Wed, 12 Jun 2024 02:09:37 +0500 Subject: [PATCH 1/7] Pass callback to client.end --- packages/pg-pool/index.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 23a7940fe..1dbf6fb1d 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -161,7 +161,7 @@ class Pool extends EventEmitter { throw new Error('unexpected condition') } - _remove(client) { + _remove(client, callback) { const removed = removeWhere(this._idle, (item) => item.client === client) if (removed !== undefined) { @@ -169,8 +169,13 @@ class Pool extends EventEmitter { } this._clients = this._clients.filter((c) => c !== client) - client.end() - this.emit('remove', client) + client.end(() => { + this.emit('remove', client) + + if (typeof callback === "function") { + callback() + } + }) } connect(cb) { @@ -351,17 +356,15 @@ class Pool extends EventEmitter { if (client._poolUseCount >= this.options.maxUses) { this.log('remove expended client') } - this._remove(client) - this._pulseQueue() - return + + return this._remove(client, this._pulseQueue.bind(this)) } const isExpired = this._expired.has(client) if (isExpired) { this.log('remove expired client') this._expired.delete(client) - this._remove(client) - this._pulseQueue() + this._remove(client, this._pulseQueue.bind(this)) return } @@ -370,7 +373,7 @@ class Pool extends EventEmitter { if (this.options.idleTimeoutMillis && this._isAboveMin()) { tid = setTimeout(() => { this.log('remove idle client') - this._remove(client) + this._remove(client, this._pulseQueue.bind(this)) }, this.options.idleTimeoutMillis) if (this.options.allowExitOnIdle) { From b0d1619fb7152ad9a1987202cfc5da47cf9c1958 Mon Sep 17 00:00:00 2001 From: Asadbek Raimov Date: Wed, 12 Jun 2024 02:10:22 +0500 Subject: [PATCH 2/7] Add test for pool.end method --- packages/pg-pool/index.js | 6 +++--- packages/pg-pool/test/ending.js | 10 ++++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 1dbf6fb1d..000d7d99d 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -172,10 +172,10 @@ class Pool extends EventEmitter { client.end(() => { this.emit('remove', client) - if (typeof callback === "function") { + if (typeof callback === 'function') { callback() } - }) + }) } connect(cb) { @@ -357,7 +357,7 @@ class Pool extends EventEmitter { this.log('remove expended client') } - return this._remove(client, this._pulseQueue.bind(this)) + return this._remove(client, this._pulseQueue.bind(this)) } const isExpired = this._expired.has(client) diff --git a/packages/pg-pool/test/ending.js b/packages/pg-pool/test/ending.js index e1839b46c..d3aa81f05 100644 --- a/packages/pg-pool/test/ending.js +++ b/packages/pg-pool/test/ending.js @@ -37,4 +37,14 @@ describe('pool ending', () => { expect(res.rows[0].name).to.equal('brianc') }) ) + + it('pool.end() - finish pending queries', async () => { + const pool = new Pool({ max: 20 }) + let completed = 0 + for (let x = 1; x <= 20; x++) { + pool.query('SELECT $1::text as name', ['brianc']).then(() => completed++) + } + await pool.end() + expect(completed).to.equal(20) + }) }) From 7e5f134752d6e7b9d1aa9bb33936ac70ed7e7782 Mon Sep 17 00:00:00 2001 From: Asadbek Raimov Date: Wed, 12 Jun 2024 12:12:17 +0500 Subject: [PATCH 3/7] fix: remove excessive _pulseQueue call --- packages/pg-pool/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 000d7d99d..821bdcb98 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -373,7 +373,7 @@ class Pool extends EventEmitter { if (this.options.idleTimeoutMillis && this._isAboveMin()) { tid = setTimeout(() => { this.log('remove idle client') - this._remove(client, this._pulseQueue.bind(this)) + this._remove(client) }, this.options.idleTimeoutMillis) if (this.options.allowExitOnIdle) { From 8e48dda8d0eb58b5805d2e4c3bcc82b812860418 Mon Sep 17 00:00:00 2001 From: Asadbek Raimov Date: Fri, 14 Jun 2024 20:56:21 +0500 Subject: [PATCH 4/7] fix: context problem --- packages/pg-pool/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 821bdcb98..aba4c1514 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -169,8 +169,9 @@ class Pool extends EventEmitter { } this._clients = this._clients.filter((c) => c !== client) + const context = this client.end(() => { - this.emit('remove', client) + context.emit('remove', client) if (typeof callback === 'function') { callback() From 018827796f938abe24fa55a03bc0b2948015c129 Mon Sep 17 00:00:00 2001 From: Asadbek Raimov Date: Tue, 18 Jun 2024 00:41:48 +0500 Subject: [PATCH 5/7] fix: test resolve should be called when the last client is removed --- packages/pg-pool/test/idle-timeout.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/pg-pool/test/idle-timeout.js b/packages/pg-pool/test/idle-timeout.js index c147af2ab..bf3b53982 100644 --- a/packages/pg-pool/test/idle-timeout.js +++ b/packages/pg-pool/test/idle-timeout.js @@ -25,6 +25,7 @@ describe('idle timeout', () => { it( 'times out and removes clients when others are also removed', co.wrap(function* () { + let currentClient = 1 const pool = new Pool({ idleTimeoutMillis: 10 }) const clientA = yield pool.connect() const clientB = yield pool.connect() @@ -35,7 +36,12 @@ describe('idle timeout', () => { pool.on('remove', () => { expect(pool.idleCount).to.equal(0) expect(pool.totalCount).to.equal(0) - resolve() + + if (currentClient >= 2) { + resolve() + } else { + currentClient++ + } }) }) From 243ffee570030cc0d8fbb97a9b1bbe2ba2a99b65 Mon Sep 17 00:00:00 2001 From: Asadbek Raimov Date: Tue, 18 Jun 2024 18:26:57 +0500 Subject: [PATCH 6/7] fix: wait for pool.end() Because when you don't pass a callback to .end() it always returns a promise --- packages/pg-pool/index.js | 5 ++--- packages/pg-pool/test/idle-timeout.js | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index aba4c1514..b3d9ada96 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -365,8 +365,7 @@ class Pool extends EventEmitter { if (isExpired) { this.log('remove expired client') this._expired.delete(client) - this._remove(client, this._pulseQueue.bind(this)) - return + return this._remove(client, this._pulseQueue.bind(this)) } // idle timeout @@ -374,7 +373,7 @@ class Pool extends EventEmitter { if (this.options.idleTimeoutMillis && this._isAboveMin()) { tid = setTimeout(() => { this.log('remove idle client') - this._remove(client) + this._remove(client, this._pulseQueue.bind(this)) }, this.options.idleTimeoutMillis) if (this.options.allowExitOnIdle) { diff --git a/packages/pg-pool/test/idle-timeout.js b/packages/pg-pool/test/idle-timeout.js index bf3b53982..4ae34429c 100644 --- a/packages/pg-pool/test/idle-timeout.js +++ b/packages/pg-pool/test/idle-timeout.js @@ -50,7 +50,7 @@ describe('idle timeout', () => { try { yield Promise.race([removal, timeout]) } finally { - pool.end() + yield pool.end() } }) ) From 289273c56f12f01414cecd0557d8bc30d8484de1 Mon Sep 17 00:00:00 2001 From: "Herman J. Radtke III" Date: Sun, 11 May 2025 07:19:34 -0400 Subject: [PATCH 7/7] fix: handle idle timeout test data race --- packages/pg-pool/test/idle-timeout.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/pg-pool/test/idle-timeout.js b/packages/pg-pool/test/idle-timeout.js index 4ae34429c..1063266d2 100644 --- a/packages/pg-pool/test/idle-timeout.js +++ b/packages/pg-pool/test/idle-timeout.js @@ -25,23 +25,25 @@ describe('idle timeout', () => { it( 'times out and removes clients when others are also removed', co.wrap(function* () { - let currentClient = 1 const pool = new Pool({ idleTimeoutMillis: 10 }) const clientA = yield pool.connect() const clientB = yield pool.connect() - clientA.release() - clientB.release(new Error()) + clientA.release() // this will put clientA in the idle pool + clientB.release(new Error()) // an error will cause clientB to be removed immediately const removal = new Promise((resolve) => { - pool.on('remove', () => { + pool.on('remove', (client) => { + // clientB's stream may take a while to close, so we may get a remove + // event for it + // we only want to handle the remove event for clientA when it times out + // due to being idle + if (client !== clientA) { + return + } + expect(pool.idleCount).to.equal(0) expect(pool.totalCount).to.equal(0) - - if (currentClient >= 2) { - resolve() - } else { - currentClient++ - } + resolve() }) }) @@ -50,7 +52,7 @@ describe('idle timeout', () => { try { yield Promise.race([removal, timeout]) } finally { - yield pool.end() + pool.end() } }) )