Skip to content

Commit 47a75c5

Browse files
authored
perf: avoid duplicate pool dispatcher selection on backpressure (#5149)
Assisted-by: openai:gpt-5.5 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
1 parent 445be40 commit 47a75c5

4 files changed

Lines changed: 122 additions & 2 deletions

File tree

lib/dispatcher/pool-base.js

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const kOnConnect = Symbol('onConnect')
1414
const kOnDisconnect = Symbol('onDisconnect')
1515
const kOnConnectionError = Symbol('onConnectionError')
1616
const kGetDispatcher = Symbol('get dispatcher')
17+
const kHasDispatcher = Symbol('has dispatcher')
1718
const kAddClient = Symbol('add client')
1819
const kRemoveClient = Symbol('remove client')
1920

@@ -162,12 +163,28 @@ class PoolBase extends DispatcherBase {
162163
this[kQueued]++
163164
} else if (!dispatcher.dispatch(opts, handler)) {
164165
dispatcher[kNeedDrain] = true
165-
this[kNeedDrain] = !this[kGetDispatcher]()
166+
this[kNeedDrain] = !this[kHasDispatcher]()
166167
}
167168

168169
return !this[kNeedDrain]
169170
}
170171

172+
[kHasDispatcher] () {
173+
for (let i = 0; i < this[kClients].length; i++) {
174+
const dispatcher = this[kClients][i]
175+
176+
if (
177+
!dispatcher[kNeedDrain] &&
178+
dispatcher.closed !== true &&
179+
dispatcher.destroyed !== true
180+
) {
181+
return true
182+
}
183+
}
184+
185+
return false
186+
}
187+
171188
[kAddClient] (client) {
172189
client
173190
.on('drain', this[kOnDrain].bind(this, client))
@@ -210,5 +227,6 @@ module.exports = {
210227
kNeedDrain,
211228
kAddClient,
212229
kRemoveClient,
213-
kGetDispatcher
230+
kGetDispatcher,
231+
kHasDispatcher
214232
}

lib/dispatcher/pool.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const {
66
kNeedDrain,
77
kAddClient,
88
kGetDispatcher,
9+
kHasDispatcher,
910
kRemoveClient
1011
} = require('./pool-base')
1112
const Client = require('./client')
@@ -115,6 +116,28 @@ class Pool extends PoolBase {
115116
return dispatcher
116117
}
117118
}
119+
120+
[kHasDispatcher] () {
121+
const clientTtlOption = this[kOptions].clientTtl
122+
for (let i = 0; i < this[kClients].length; i++) {
123+
const client = this[kClients][i]
124+
125+
if (clientTtlOption != null && clientTtlOption > 0 && client.ttl && ((Date.now() - client.ttl) > clientTtlOption)) {
126+
this[kRemoveClient](client)
127+
i--
128+
} else if (!client[kNeedDrain]) {
129+
return true
130+
}
131+
}
132+
133+
if (!this[kConnections] || this[kClients].length < this[kConnections]) {
134+
const dispatcher = this[kFactory](this[kUrl], this[kOptions])
135+
this[kAddClient](dispatcher)
136+
return true
137+
}
138+
139+
return false
140+
}
118141
}
119142

120143
module.exports = Pool

lib/dispatcher/round-robin-pool.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const {
66
kNeedDrain,
77
kAddClient,
88
kGetDispatcher,
9+
kHasDispatcher,
910
kRemoveClient
1011
} = require('./pool-base')
1112
const Client = require('./client')
@@ -128,6 +129,31 @@ class RoundRobinPool extends PoolBase {
128129
return dispatcher
129130
}
130131
}
132+
133+
[kHasDispatcher] () {
134+
const clientTtlOption = this[kOptions].clientTtl
135+
for (let i = 0; i < this[kClients].length; i++) {
136+
const client = this[kClients][i]
137+
138+
if (clientTtlOption != null && clientTtlOption > 0 && client.ttl && ((Date.now() - client.ttl) > clientTtlOption)) {
139+
this[kRemoveClient](client)
140+
if (i <= this[kIndex]) {
141+
this[kIndex]--
142+
}
143+
i--
144+
} else if (!client[kNeedDrain]) {
145+
return true
146+
}
147+
}
148+
149+
if (!this[kConnections] || this[kClients].length < this[kConnections]) {
150+
const dispatcher = this[kFactory](this[kUrl], this[kOptions])
151+
this[kAddClient](dispatcher)
152+
return true
153+
}
154+
155+
return false
156+
}
131157
}
132158

133159
module.exports = RoundRobinPool

test/node-test/balanced-pool.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@
33
const { describe, test } = require('node:test')
44
const assert = require('node:assert/strict')
55
const { BalancedPool, Pool, Client, errors } = require('../..')
6+
const { EventEmitter } = require('node:events')
67
const { createServer } = require('node:http')
78
const { promisify } = require('node:util')
89
const { tspl } = require('@matteo.collina/tspl')
10+
const { kUrl } = require('../../lib/core/symbols')
11+
const { kGetDispatcher } = require('../../lib/dispatcher/pool-base')
912

1013
test('throws when factory is not a function', (t) => {
1114
const p = tspl(t, { plan: 2 })
@@ -48,6 +51,56 @@ test('add/remove upstreams', (t) => {
4851
p.deepStrictEqual(pool.upstreams, [])
4952
})
5053

54+
test('does not select dispatcher twice when selected dispatcher backpressures', (t) => {
55+
class FakeDispatcher extends EventEmitter {
56+
constructor (origin) {
57+
super()
58+
this[kUrl] = new URL(origin)
59+
}
60+
61+
dispatch () {
62+
return false
63+
}
64+
65+
close () {
66+
this.closed = true
67+
return Promise.resolve()
68+
}
69+
70+
destroy () {
71+
this.destroyed = true
72+
return Promise.resolve()
73+
}
74+
}
75+
76+
class CountingBalancedPool extends BalancedPool {
77+
constructor (...args) {
78+
super(...args)
79+
this.calls = 0
80+
}
81+
82+
[kGetDispatcher] () {
83+
this.calls++
84+
return super[kGetDispatcher]()
85+
}
86+
}
87+
88+
const pool = new CountingBalancedPool([
89+
'http://localhost:1',
90+
'http://localhost:2'
91+
], {
92+
factory: (origin) => new FakeDispatcher(origin)
93+
})
94+
t.after(() => pool.close())
95+
96+
const ret = pool.dispatch({}, {
97+
onResponseError () {}
98+
})
99+
100+
assert.strictEqual(ret, true)
101+
assert.strictEqual(pool.calls, 1)
102+
})
103+
51104
test('basic get', async (t) => {
52105
const p = tspl(t, { plan: 16 })
53106

0 commit comments

Comments
 (0)