Skip to content

Commit 41fc61f

Browse files
committed
Make connection aquisition timeout also consider the connection creation time
The connection acquisition timeout must account for the whole acquisition execution time, whether a new connection is created, an idle connection is picked up instead or we need to wait until the full pool depletes. In particular, the connection acquisition timeout (CAT) has precedence over the socket connection timeout (SCT). If the SCT is set to 2 hours and CAT to 50ms, the connection acquisition should time out after 50ms (as usual, these evil cats win), even if the connection is successfully created within the SCT period. Note: if SCT is larger than or equal to CAT, a warning should be issued, as this is likely a misconfiguration (and big SCT values won't have any effect on connection acquisitions anyway).
1 parent 92fbe79 commit 41fc61f

File tree

2 files changed

+41
-41
lines changed

2 files changed

+41
-41
lines changed

packages/bolt-connection/src/pool/pool.js

Lines changed: 37 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -74,51 +74,47 @@ class Pool {
7474
* @return {Object} resource that is ready to use.
7575
*/
7676
acquire (address) {
77-
return this._acquire(address).then(resource => {
78-
const key = address.asKey()
7977

80-
if (resource) {
81-
// New or existing resource acquired
82-
return resource
83-
}
78+
const key = address.asKey()
8479

85-
// We're out of resources and will try to acquire later on when an existing resource is released.
86-
const allRequests = this._acquireRequests
87-
const requests = allRequests[key]
88-
if (!requests) {
89-
allRequests[key] = []
90-
}
80+
// We're out of resources and will try to acquire later on when an existing resource is released.
81+
const allRequests = this._acquireRequests
82+
const requests = allRequests[key]
83+
if (!requests) {
84+
allRequests[key] = []
85+
}
9186

92-
return new Promise((resolve, reject) => {
93-
let request
94-
95-
const timeoutId = setTimeout(() => {
96-
// acquisition timeout fired
97-
98-
// remove request from the queue of pending requests, if it's still there
99-
// request might've been taken out by the release operation
100-
const pendingRequests = allRequests[key]
101-
if (pendingRequests) {
102-
allRequests[key] = pendingRequests.filter(item => item !== request)
103-
}
104-
105-
if (request.isCompleted()) {
106-
// request already resolved/rejected by the release operation; nothing to do
107-
} else {
108-
// request is still pending and needs to be failed
109-
const activeCount = this.activeResourceCount(address)
110-
const idleCount = this.has(address) ? this._pools[key].length : 0
111-
request.reject(
112-
newError(
113-
`Connection acquisition timed out in ${this._acquisitionTimeout} ms. Pool status: Active conn count = ${activeCount}, Idle conn count = ${idleCount}.`
114-
)
87+
return new Promise((resolve, reject) => {
88+
let request
89+
90+
const timeoutId = setTimeout(() => {
91+
// acquisition timeout fired
92+
93+
// remove request from the queue of pending requests, if it's still there
94+
// request might've been taken out by the release operation
95+
const pendingRequests = allRequests[key]
96+
if (pendingRequests) {
97+
allRequests[key] = pendingRequests.filter(item => item !== request)
98+
}
99+
100+
if (request.isCompleted()) {
101+
// request already resolved/rejected by the release operation; nothing to do
102+
} else {
103+
// request is still pending and needs to be failed
104+
const activeCount = this.activeResourceCount(address)
105+
const idleCount = this.has(address) ? this._pools[key].length : 0
106+
request.reject(
107+
newError(
108+
`Connection acquisition timed out in ${this._acquisitionTimeout} ms. Pool status: Active conn count = ${activeCount}, Idle conn count = ${idleCount}.`
115109
)
116-
}
117-
}, this._acquisitionTimeout)
110+
)
111+
}
112+
}, this._acquisitionTimeout)
113+
114+
request = new PendingRequest(key, resolve, reject, timeoutId, this._log)
115+
allRequests[key].push(request)
118116

119-
request = new PendingRequest(key, resolve, reject, timeoutId, this._log)
120-
allRequests[key].push(request)
121-
})
117+
this._processPendingAcquireRequests(address)
122118
})
123119
}
124120

@@ -315,7 +311,7 @@ class Pool {
315311
_processPendingAcquireRequests (address) {
316312
const key = address.asKey()
317313
const requests = this._acquireRequests[key]
318-
const poolState = this._poolState[key]
314+
const poolState = this._poolState[key] || new PoolState()
319315
if (requests) {
320316
const pendingRequest = requests.shift() // pop a pending acquire request
321317

packages/testkit-backend/src/request-handlers.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ export function NewDriver (context, data, wire) {
8585
if ('connectionAcquisitionTimeoutMs' in data) {
8686
config.connectionAcquisitionTimeout = data.connectionAcquisitionTimeoutMs
8787
}
88+
if ('connectionTimeoutMs' in data) {
89+
config.connectionTimeout = data.connectionTimeoutMs
90+
}
8891
if ('fetchSize' in data) {
8992
config.fetchSize = data.fetchSize
9093
}
@@ -380,6 +383,7 @@ export function GetFeatures (_context, _params, wire) {
380383
'Feature:Bolt:4.4',
381384
'Feature:API:Result.List',
382385
'Feature:API:Result.Peek',
386+
'Feature:Configuruation:ConnectionAcquisitionTimeout',
383387
'Optimization:EagerTransactionBegin',
384388
'Optimization:ImplicitDefaultArguments',
385389
'Optimization:PullPipelining',

0 commit comments

Comments
 (0)