From 48d1767b9dc4e32bd5138a28ee72db59b6daae32 Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 4 Mar 2025 11:48:24 +0100 Subject: [PATCH 01/17] perf(pg-pool): optimize client retrieval from pool by prioritizing free client with cached prepared statements see https://github.com/brianc/node-postgres/issues/3397 --- packages/pg-pool/index.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index e7d2d8833..dc4517b73 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -111,7 +111,7 @@ class Pool extends EventEmitter { return this._clients.length >= this.options.max } - _pulseQueue() { + _pulseQueue(name) { this.log('pulse queue') if (this.ended) { this.log('pulse queue ended') @@ -142,7 +142,10 @@ class Pool extends EventEmitter { } const pendingItem = this._pendingQueue.shift() if (this._idle.length) { - const idleItem = this._idle.pop() + let idleItem = name ? this._idle.find((item) => item.client.connection.parsedStatements[name]) : null + if (!idleItem) { + idleItem = this._idle.pop() + } clearTimeout(idleItem.timeoutId) const client = idleItem.client client.ref && client.ref() @@ -168,7 +171,7 @@ class Pool extends EventEmitter { this.emit('remove', client) } - connect(cb) { + connect(cb, name) { if (this.ending) { const err = new Error('Cannot use a pool after calling end on the pool') return cb ? cb(err) : this.Promise.reject(err) @@ -181,7 +184,7 @@ class Pool extends EventEmitter { if (this._isFull() || this._idle.length) { // if we have idle clients schedule a pulse immediately if (this._idle.length) { - process.nextTick(() => this._pulseQueue()) + process.nextTick(() => this._pulseQueue(name)) } if (!this.options.connectionTimeoutMillis) { @@ -431,7 +434,7 @@ class Pool extends EventEmitter { client.release(err) return cb(err) } - }) + }, text.name) return response.result } From 8b382a621c81fa31ff0f5ae1abd182d737292557 Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 4 Mar 2025 11:55:56 +0100 Subject: [PATCH 02/17] fix: test --- 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 dc4517b73..8c05cb3ce 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -434,7 +434,7 @@ class Pool extends EventEmitter { client.release(err) return cb(err) } - }, text.name) + }, text?.name) return response.result } From 498d7f47e6877a941fed99ab04e346d3d3bb3e45 Mon Sep 17 00:00:00 2001 From: francesco Date: Wed, 5 Mar 2025 09:02:43 +0100 Subject: [PATCH 03/17] fix: remove the element found --- packages/pg-pool/index.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 8c05cb3ce..7824a56f8 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -142,7 +142,14 @@ class Pool extends EventEmitter { } const pendingItem = this._pendingQueue.shift() if (this._idle.length) { - let idleItem = name ? this._idle.find((item) => item.client.connection.parsedStatements[name]) : null + let idleItem = null + if (name) { + // find if there is a free client that already has cached prepared statements + const index = this._idle.findIndex((item) => item.client.connection.parsedStatements[name]) + if (index > -1) { + idleItem = this._idle.splice(index, 1)[0] + } + } if (!idleItem) { idleItem = this._idle.pop() } From e938db2bbc188d3b7f3b6a0f50bf9fc05814c916 Mon Sep 17 00:00:00 2001 From: francesco Date: Wed, 5 Mar 2025 09:04:22 +0100 Subject: [PATCH 04/17] fix: lint --- 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 7824a56f8..69dc00c2a 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -149,7 +149,7 @@ class Pool extends EventEmitter { if (index > -1) { idleItem = this._idle.splice(index, 1)[0] } - } + } if (!idleItem) { idleItem = this._idle.pop() } From ae71ac7bc9dda102d5804b95a6ebfd88009e2aaa Mon Sep 17 00:00:00 2001 From: francesco Date: Wed, 5 Mar 2025 09:19:23 +0100 Subject: [PATCH 05/17] feat: add test --- .../test/prioritizing-prepared-client.js | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 packages/pg-pool/test/prioritizing-prepared-client.js diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js new file mode 100644 index 000000000..04328cdfc --- /dev/null +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -0,0 +1,29 @@ +const expect = require('expect.js') +const co = require('co') + +const describe = require('mocha').describe +const it = require('mocha').it + +const Pool = require('../') + +describe('prioritizing prepared client', () => { + it('can create a single client with prepared statment and reuse it', + co.wrap(function* () { + const pool = new Pool({ max: 2 }) + expect(pool.waitingCount).to.equal(0) + + let res + + res = yield pool.query({text: 'SELECT $1::text as name', values: ['hi'], name: 'foo'}) + expect(res.rows[0].name).to.equal('hi') + expect(pool._idle.length).to.equal(1) + + res = yield pool.query({text: 'SELECT $1::text as name', values: ['hi'], name: 'foo'}) + expect(res.rows[0].name).to.equal('hi') + expect(pool._idle.length).to.equal(1) + + pool.end() + }) + ) + +}) From 1054ce79d4eef0a3512db9a637ec34ee708c7655 Mon Sep 17 00:00:00 2001 From: francesco Date: Wed, 5 Mar 2025 09:22:30 +0100 Subject: [PATCH 06/17] fix: lint --- packages/pg-pool/test/prioritizing-prepared-client.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index 04328cdfc..3801e7013 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -7,19 +7,20 @@ const it = require('mocha').it const Pool = require('../') describe('prioritizing prepared client', () => { - it('can create a single client with prepared statment and reuse it', + it( + 'can create a single client with prepared statment and reuse it', co.wrap(function* () { const pool = new Pool({ max: 2 }) expect(pool.waitingCount).to.equal(0) let res - + res = yield pool.query({text: 'SELECT $1::text as name', values: ['hi'], name: 'foo'}) expect(res.rows[0].name).to.equal('hi') expect(pool._idle.length).to.equal(1) - - res = yield pool.query({text: 'SELECT $1::text as name', values: ['hi'], name: 'foo'}) - expect(res.rows[0].name).to.equal('hi') + + res = yield pool.query({text: 'SELECT $1::text as name', values: ['ho'], name: 'foo'}) + expect(res.rows[0].name).to.equal('ho') expect(pool._idle.length).to.equal(1) pool.end() From 4abb64d39479bd44a17c55fe3d7227a42db5f806 Mon Sep 17 00:00:00 2001 From: francesco Date: Wed, 5 Mar 2025 09:24:45 +0100 Subject: [PATCH 07/17] fix: lint --- packages/pg-pool/test/prioritizing-prepared-client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index 3801e7013..84e23465b 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -15,11 +15,11 @@ describe('prioritizing prepared client', () => { let res - res = yield pool.query({text: 'SELECT $1::text as name', values: ['hi'], name: 'foo'}) + res = yield pool.query({ text: 'SELECT $1::text as name', values: ['hi'], name: 'foo' }) expect(res.rows[0].name).to.equal('hi') expect(pool._idle.length).to.equal(1) - res = yield pool.query({text: 'SELECT $1::text as name', values: ['ho'], name: 'foo'}) + res = yield pool.query({ text: 'SELECT $1::text as name', values: ['ho'], name: 'foo' }) expect(res.rows[0].name).to.equal('ho') expect(pool._idle.length).to.equal(1) From 754b85d844a351102e0112557e7c422ed5d509d5 Mon Sep 17 00:00:00 2001 From: francesco Date: Wed, 5 Mar 2025 09:26:04 +0100 Subject: [PATCH 08/17] fix: lint --- packages/pg-pool/test/prioritizing-prepared-client.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index 84e23465b..e45ddd48c 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -26,5 +26,4 @@ describe('prioritizing prepared client', () => { pool.end() }) ) - }) From 20e7fa2fcc3b9c48b03a8661dc71f8284d50f0e1 Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 11 Mar 2025 09:00:43 +0100 Subject: [PATCH 09/17] chore: check if is the same pid --- packages/pg-pool/test/prioritizing-prepared-client.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index e45ddd48c..99790b28f 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -15,13 +15,17 @@ describe('prioritizing prepared client', () => { let res - res = yield pool.query({ text: 'SELECT $1::text as name', values: ['hi'], name: 'foo' }) + res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['hi'], name: 'foo' }) expect(res.rows[0].name).to.equal('hi') expect(pool._idle.length).to.equal(1) + const firstPid = res.rows[0].pid - res = yield pool.query({ text: 'SELECT $1::text as name', values: ['ho'], name: 'foo' }) + res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['ho'], name: 'foo' }) expect(res.rows[0].name).to.equal('ho') expect(pool._idle.length).to.equal(1) + const secondPid = res.rows[0].pid + + expect(firstPid).to.equal(secondPid) pool.end() }) From 529e90b2c3c52b12a1e9ba9cc1b1a7974c47517b Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 11 Mar 2025 11:23:35 +0100 Subject: [PATCH 10/17] chore: force two idle client --- packages/pg-pool/test/prioritizing-prepared-client.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index 99790b28f..9bcda3807 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -12,6 +12,17 @@ describe('prioritizing prepared client', () => { co.wrap(function* () { const pool = new Pool({ max: 2 }) expect(pool.waitingCount).to.equal(0) + expect(pool._idle.length).to.equal(0) + + // force the creation of two client and release. + // In this way we have two idle client + const firstClient = yield pool.connect() + expect(pool._clients.length).to.equal(1) + const secondClient = yield pool.connect() + expect(pool._clients.length).to.equal(2) + firstClient.release() + secondClient.release() + expect(pool._idle.length).to.equal(2) let res From b79c98803b8454b14f15f986cb60ad2584f4d69e Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 11 Mar 2025 11:45:23 +0100 Subject: [PATCH 11/17] fix: lint --- packages/pg-pool/test/prioritizing-prepared-client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index 9bcda3807..59dbc0a03 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -14,7 +14,7 @@ describe('prioritizing prepared client', () => { expect(pool.waitingCount).to.equal(0) expect(pool._idle.length).to.equal(0) - // force the creation of two client and release. + // force the creation of two client and release. // In this way we have two idle client const firstClient = yield pool.connect() expect(pool._clients.length).to.equal(1) From f65523c36858142780ba33777b9b760815290728 Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 11 Mar 2025 11:49:40 +0100 Subject: [PATCH 12/17] fix: wrong check --- packages/pg-pool/test/prioritizing-prepared-client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index 59dbc0a03..d8923cf3d 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -28,12 +28,12 @@ describe('prioritizing prepared client', () => { res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['hi'], name: 'foo' }) expect(res.rows[0].name).to.equal('hi') - expect(pool._idle.length).to.equal(1) + expect(pool._idle.length).to.equal(2) const firstPid = res.rows[0].pid res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['ho'], name: 'foo' }) expect(res.rows[0].name).to.equal('ho') - expect(pool._idle.length).to.equal(1) + expect(pool._idle.length).to.equal(2) const secondPid = res.rows[0].pid expect(firstPid).to.equal(secondPid) From 0cb01b84d264f15bbcee5e2c803da2d48170cde7 Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 11 Mar 2025 11:56:28 +0100 Subject: [PATCH 13/17] chore: not the same client without prepared query --- .../test/prioritizing-prepared-client.js | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index d8923cf3d..6c04a3c9d 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -24,20 +24,36 @@ describe('prioritizing prepared client', () => { secondClient.release() expect(pool._idle.length).to.equal(2) - let res + let res, firstPid, secondPid + + // check the same client with prepared query res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['hi'], name: 'foo' }) expect(res.rows[0].name).to.equal('hi') expect(pool._idle.length).to.equal(2) - const firstPid = res.rows[0].pid + firstPid = res.rows[0].pid res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['ho'], name: 'foo' }) expect(res.rows[0].name).to.equal('ho') expect(pool._idle.length).to.equal(2) - const secondPid = res.rows[0].pid + secondPid = res.rows[0].pid expect(firstPid).to.equal(secondPid) + // not the same client without prepared query + + res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['hi'] }) + expect(res.rows[0].name).to.equal('hi') + expect(pool._idle.length).to.equal(2) + firstPid = res.rows[0].pid + + res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['ho'] }) + expect(res.rows[0].name).to.equal('ho') + expect(pool._idle.length).to.equal(2) + secondPid = res.rows[0].pid + + expect(firstPid).to.not.equal(secondPid) + pool.end() }) ) From 975e89106bfbafd673557f957c44a17acebe2fd8 Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 11 Mar 2025 12:01:51 +0100 Subject: [PATCH 14/17] fix: delete wrong test --- .../pg-pool/test/prioritizing-prepared-client.js | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index 6c04a3c9d..bec11df8f 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -40,20 +40,6 @@ describe('prioritizing prepared client', () => { expect(firstPid).to.equal(secondPid) - // not the same client without prepared query - - res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['hi'] }) - expect(res.rows[0].name).to.equal('hi') - expect(pool._idle.length).to.equal(2) - firstPid = res.rows[0].pid - - res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['ho'] }) - expect(res.rows[0].name).to.equal('ho') - expect(pool._idle.length).to.equal(2) - secondPid = res.rows[0].pid - - expect(firstPid).to.not.equal(secondPid) - pool.end() }) ) From 74b5da45e108a7766de93304a14b2f5c326c03ce Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 11 Mar 2025 14:06:22 +0100 Subject: [PATCH 15/17] chore: test connection with name --- packages/pg-pool/index.js | 6 ++++++ .../test/prioritizing-prepared-client.js | 17 +++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 69dc00c2a..0bc3ddec9 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -179,6 +179,12 @@ class Pool extends EventEmitter { } connect(cb, name) { + // guard clause against passing a name as the first parameter + if (typeof cb === 'string') { + name = cb; + cb = undefined; + } + if (this.ending) { const err = new Error('Cannot use a pool after calling end on the pool') return cb ? cb(err) : this.Promise.reject(err) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index bec11df8f..f6c88b48c 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -14,18 +14,18 @@ describe('prioritizing prepared client', () => { expect(pool.waitingCount).to.equal(0) expect(pool._idle.length).to.equal(0) + let res, firstClient, secondClient, firstPid, secondPid + // force the creation of two client and release. // In this way we have two idle client - const firstClient = yield pool.connect() + firstClient = yield pool.connect() expect(pool._clients.length).to.equal(1) - const secondClient = yield pool.connect() + secondClient = yield pool.connect() expect(pool._clients.length).to.equal(2) firstClient.release() secondClient.release() expect(pool._idle.length).to.equal(2) - let res, firstPid, secondPid - // check the same client with prepared query res = yield pool.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['hi'], name: 'foo' }) @@ -40,6 +40,15 @@ describe('prioritizing prepared client', () => { expect(firstPid).to.equal(secondPid) + // check also connect with name, return same client + + firstClient = yield pool.connect('foo') + res = yield firstClient.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['hi'] }) + expect(res.rows[0].name).to.equal('hi') + expect(firstPid).to.not.equal(res.rows[0].pid) + firstClient.release() + expect(pool._idle.length).to.equal(2) + pool.end() }) ) From e7879300e6fe381162ad339cfd79e82ebac2c387 Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 11 Mar 2025 14:07:45 +0100 Subject: [PATCH 16/17] fix: lint --- packages/pg-pool/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pg-pool/index.js b/packages/pg-pool/index.js index 0bc3ddec9..18c906da2 100644 --- a/packages/pg-pool/index.js +++ b/packages/pg-pool/index.js @@ -181,8 +181,8 @@ class Pool extends EventEmitter { connect(cb, name) { // guard clause against passing a name as the first parameter if (typeof cb === 'string') { - name = cb; - cb = undefined; + name = cb + cb = undefined } if (this.ending) { From 34b9a1a5517d63f6b074a70a4fb7c3df11115ad9 Mon Sep 17 00:00:00 2001 From: francesco Date: Tue, 11 Mar 2025 14:26:04 +0100 Subject: [PATCH 17/17] fix: test --- packages/pg-pool/test/prioritizing-prepared-client.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/pg-pool/test/prioritizing-prepared-client.js b/packages/pg-pool/test/prioritizing-prepared-client.js index f6c88b48c..376ba77b9 100644 --- a/packages/pg-pool/test/prioritizing-prepared-client.js +++ b/packages/pg-pool/test/prioritizing-prepared-client.js @@ -43,9 +43,10 @@ describe('prioritizing prepared client', () => { // check also connect with name, return same client firstClient = yield pool.connect('foo') + expect(pool._idle.length).to.equal(1) res = yield firstClient.query({ text: 'SELECT $1::text as name, pg_backend_pid() as pid', values: ['hi'] }) expect(res.rows[0].name).to.equal('hi') - expect(firstPid).to.not.equal(res.rows[0].pid) + expect(firstPid).to.equal(res.rows[0].pid) firstClient.release() expect(pool._idle.length).to.equal(2)