Skip to content

Commit faad920

Browse files
TimelordUKTimelordUK
committed
ensure we dont grow the pool if we already have enough connections to service the work.
1 parent 2822030 commit faad920

File tree

2 files changed

+249
-15
lines changed

2 files changed

+249
-15
lines changed

‎lib/pool.js‎

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -433,21 +433,44 @@ const poolModule = (() => {
433433
if (closed) {
434434
return
435435
}
436-
void grow().then(() => {
437-
promotePause()
438-
while (workQueue.length > 0 && idle.length > 0) {
439-
const work = workQueue.pop()
440-
if (work.poolNotifier.isPendingCancel()) {
441-
_this.emit('debug', `query work id = ${work.id} has been cancelled waiting in pool to execute, workQueue = ${workQueue.length}`)
442-
doneFree(work.poolNotifier)
443-
} else if (work.poolNotifier.isPaused()) {
444-
pause.unshift(work)
445-
} else {
446-
const description = checkout('work')
447-
item(description, work)
448-
}
436+
437+
// First, promote any unpaused items from pause queue
438+
promotePause()
439+
440+
// Process work with existing idle connections
441+
while (workQueue.length > 0 && idle.length > 0) {
442+
const work = workQueue.pop()
443+
if (work.poolNotifier.isPendingCancel()) {
444+
_this.emit('debug', `query work id = ${work.id} has been cancelled waiting in pool to execute, workQueue = ${workQueue.length}`)
445+
doneFree(work.poolNotifier)
446+
} else if (work.poolNotifier.isPaused()) {
447+
pause.unshift(work)
448+
} else {
449+
const description = checkout('work')
450+
item(description, work)
449451
}
450-
})
452+
}
453+
454+
// Only grow the pool if we still have work queued and no idle connections
455+
if (workQueue.length > 0 && idle.length === 0) {
456+
_this.emit('debug', `crank needs to grow pool: workQueue = ${workQueue.length}, idle = ${idle.length}, busy = ${busyConnectionCount}`)
457+
void grow().then(() => {
458+
// After growing, process any remaining work
459+
promotePause()
460+
while (workQueue.length > 0 && idle.length > 0) {
461+
const work = workQueue.pop()
462+
if (work.poolNotifier.isPendingCancel()) {
463+
_this.emit('debug', `query work id = ${work.id} has been cancelled waiting in pool to execute, workQueue = ${workQueue.length}`)
464+
doneFree(work.poolNotifier)
465+
} else if (work.poolNotifier.isPaused()) {
466+
pause.unshift(work)
467+
} else {
468+
const description = checkout('work')
469+
item(description, work)
470+
}
471+
}
472+
})
473+
}
451474
}
452475

453476
const workTypeEnum = {
@@ -640,6 +663,12 @@ const poolModule = (() => {
640663
if (existing >= options.ceiling) {
641664
return
642665
}
666+
667+
// For initial pool creation, grow to floor if not yet reached
668+
const targetSize = opened ? options.ceiling : options.floor
669+
if (existing >= targetSize) {
670+
return
671+
}
643672

644673
function connectionOptions (c) {
645674
c.setSharedCache(poolProcedureCache, poolTableCache)
@@ -656,7 +685,7 @@ const poolModule = (() => {
656685

657686
// Calculate how many connections to create based on strategy
658687
let connectionsToCreate = 0
659-
const needed = options.ceiling - existing
688+
const needed = targetSize - existing
660689

661690
switch (options.scalingStrategy) {
662691
case 'gradual':

‎test/pool-growth-efficiency.test.js‎

Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
'use strict'
2+
3+
/* global describe after it */
4+
5+
const assert = require('assert')
6+
const { TestEnv } = require('./env/test-env')
7+
8+
describe('pool growth efficiency', function () {
9+
this.timeout(30000)
10+
const env = new TestEnv()
11+
const sql = env.sql
12+
13+
this.beforeEach(done => {
14+
env.open().then(() => done()).catch(done)
15+
})
16+
17+
this.afterEach(done => {
18+
env.close().then(() => done()).catch(done)
19+
})
20+
21+
it('pool should not grow when idle connections are available', async function handler () {
22+
const debugMessages = []
23+
const growthMessages = []
24+
const crankMessages = []
25+
26+
const pool = new sql.Pool({
27+
connectionString: env.connectionString,
28+
floor: 10,
29+
ceiling: 20,
30+
heartbeatSecs: 1,
31+
inactivityTimeoutSecs: 10
32+
})
33+
34+
// Capture debug messages to verify behavior
35+
pool.on('debug', msg => {
36+
debugMessages.push(msg)
37+
console.log('[DEBUG]', msg)
38+
if (msg.includes('grow creates')) {
39+
growthMessages.push(msg)
40+
}
41+
if (msg.includes('crank needs to grow pool')) {
42+
crankMessages.push(msg)
43+
}
44+
})
45+
46+
await pool.promises.open()
47+
48+
// Wait a moment to ensure all initial connections are established
49+
await new Promise(resolve => setTimeout(resolve, 500))
50+
51+
// Submit 10 queries (matching the floor/initial connections)
52+
const queries = []
53+
for (let i = 0; i < 10; i++) {
54+
queries.push(pool.promises.query(`SELECT ${i} as num, @@SPID as spid`))
55+
}
56+
57+
// Wait for all queries to complete
58+
const results1 = await Promise.all(queries)
59+
assert.strictEqual(results1.length, 10)
60+
61+
// Wait a moment for connections to return to idle
62+
await new Promise(resolve => setTimeout(resolve, 100))
63+
64+
// Submit another batch of 10 queries
65+
const queries2 = []
66+
for (let i = 10; i < 20; i++) {
67+
queries2.push(pool.promises.query(`SELECT ${i} as num, @@SPID as spid`))
68+
}
69+
70+
const results2 = await Promise.all(queries2)
71+
assert.strictEqual(results2.length, 10)
72+
73+
await pool.promises.close()
74+
75+
// Debug output
76+
console.log('\n=== Test Summary ===')
77+
console.log('Growth messages:', growthMessages)
78+
console.log('Crank messages:', crankMessages)
79+
console.log('Total growth events:', growthMessages.length)
80+
console.log('Total crank growth requests:', crankMessages.length)
81+
82+
// Verify behavior - the pool may grow in stages during initial open
83+
const totalGrowthConnections = growthMessages.reduce((sum, msg) => {
84+
const match = msg.match(/grow creates (\d+) connections/)
85+
return sum + (match ? parseInt(match[1]) : 0)
86+
}, 0)
87+
88+
console.log('Total connections created:', totalGrowthConnections)
89+
90+
// We expect the pool to create connections up to the floor (10)
91+
assert(totalGrowthConnections >= 10, `Should have created at least floor (10) connections, but created ${totalGrowthConnections}`)
92+
93+
// The key test: after initial setup, no additional growth should occur
94+
// when we're just reusing idle connections
95+
assert.strictEqual(crankMessages.length, 0, 'Should not have needed to grow pool when idle connections were available')
96+
})
97+
98+
it('pool should grow only when all connections are busy', async function handler () {
99+
const debugMessages = []
100+
const growthMessages = []
101+
const crankMessages = []
102+
103+
const pool = new sql.Pool({
104+
connectionString: env.connectionString,
105+
floor: 2,
106+
ceiling: 10,
107+
scalingStrategy: 'gradual',
108+
scalingIncrement: 2
109+
})
110+
111+
pool.on('debug', msg => {
112+
debugMessages.push(msg)
113+
console.log('[DEBUG]', msg)
114+
if (msg.includes('grow creates')) {
115+
growthMessages.push(msg)
116+
}
117+
if (msg.includes('crank needs to grow pool')) {
118+
crankMessages.push(msg)
119+
}
120+
})
121+
122+
await pool.promises.open()
123+
124+
// Wait for initial connections
125+
await new Promise(resolve => setTimeout(resolve, 500))
126+
127+
// Submit queries that will hold connections busy
128+
const slowQueries = []
129+
for (let i = 0; i < 5; i++) {
130+
slowQueries.push(pool.promises.query("WAITFOR DELAY '00:00:01'; SELECT @@SPID as spid"))
131+
}
132+
133+
// Give a moment for queries to start
134+
await new Promise(resolve => setTimeout(resolve, 100))
135+
136+
// Wait for queries to complete
137+
await Promise.all(slowQueries)
138+
await pool.promises.close()
139+
140+
console.log('\n=== Test Summary ===')
141+
console.log('Growth messages:', growthMessages)
142+
console.log('Crank growth messages:', crankMessages)
143+
144+
// Verify that growth occurred when needed
145+
// We submitted 5 slow queries but only have 2 initial connections
146+
// So the pool should have grown
147+
assert(crankMessages.length > 0 || growthMessages.length > 1, 'Pool should grow when all connections are busy')
148+
})
149+
150+
it('pool with burst workload should reuse connections efficiently', async function handler () {
151+
const spidsUsed = new Set()
152+
const debugMessages = []
153+
154+
const pool = new sql.Pool({
155+
connectionString: env.connectionString,
156+
floor: 5,
157+
ceiling: 20
158+
})
159+
160+
pool.on('debug', msg => {
161+
debugMessages.push(msg)
162+
if (msg.includes('checkout') || msg.includes('checkin') || msg.includes('grow')) {
163+
console.log('[DEBUG]', msg)
164+
}
165+
})
166+
167+
await pool.promises.open()
168+
169+
// Wait for initial connections
170+
await new Promise(resolve => setTimeout(resolve, 1000))
171+
172+
// Burst 1: Submit 5 queries
173+
console.log('\nBurst 1: 5 queries')
174+
const burst1 = []
175+
for (let i = 0; i < 5; i++) {
176+
burst1.push(pool.promises.query('SELECT @@SPID as spid'))
177+
}
178+
179+
const results1 = await Promise.all(burst1)
180+
results1.forEach(r => spidsUsed.add(r.first[0].spid))
181+
console.log('SPIDs after burst 1:', Array.from(spidsUsed))
182+
183+
// Wait for connections to return to idle
184+
await new Promise(resolve => setTimeout(resolve, 500))
185+
186+
// Burst 2: Submit another 5 queries
187+
console.log('\nBurst 2: 5 queries')
188+
const burst2 = []
189+
for (let i = 0; i < 5; i++) {
190+
burst2.push(pool.promises.query('SELECT @@SPID as spid'))
191+
}
192+
193+
const results2 = await Promise.all(burst2)
194+
const spidsBeforeBurst2 = spidsUsed.size
195+
results2.forEach(r => spidsUsed.add(r.first[0].spid))
196+
console.log('SPIDs after burst 2:', Array.from(spidsUsed))
197+
console.log('New SPIDs in burst 2:', spidsUsed.size - spidsBeforeBurst2)
198+
199+
await pool.promises.close()
200+
201+
// Verify that we reused connections (should have at most 5 unique SPIDs)
202+
console.log('\nUnique SPIDs used:', spidsUsed.size)
203+
assert(spidsUsed.size <= 5, `Should reuse connections efficiently, but used ${spidsUsed.size} unique connections`)
204+
})
205+
})

0 commit comments

Comments
 (0)