Skip to content

Commit e87c70c

Browse files
committed
Improve release of broken connection when pool is full
Release operation discards the connection if it is broken. It also tries to resolve a pending acquisition request, if any. In order to this it tries to execute a promise-free internal acquire that immediately returns an available resource or creates a new one, if possible. New resource can only be created if pool is not at its maximum capacity. Previously, release operation only decremented the active connection counter at the end. This made it impossible to create a new connection when broken one is returned. This commit fixes the problem by making `Pool#_release()` decrement the active counter after connection has been either added to the list of idle connections or discarded because it turned out to be broken.
1 parent 0dd57b4 commit e87c70c

File tree

2 files changed

+36
-5
lines changed

2 files changed

+36
-5
lines changed

src/v1/internal/pool.js

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ class Pool {
154154
// key has been purged, don't put it back, just destroy the resource
155155
this._destroy(resource);
156156
}
157+
resourceReleased(key, this._activeResourceCounts);
157158

158159
// check if there are any pending requests
159160
const requests = this._acquireRequests[key];
@@ -164,18 +165,14 @@ class Pool {
164165
const resource = this._acquire(key);
165166
if (resource) {
166167
// managed to acquire a valid resource from the pool to satisfy the pending acquire request
168+
resourceAcquired(key, this._activeResourceCounts); // increment the active counter
167169
requests.shift(); // forget the pending request
168170
pending.resolve(resource); // resolve the pending request with the acquired resource
169-
170-
// return early to not decrement the number of released resources
171-
return;
172171
}
173172
} else {
174173
delete this._acquireRequests[key];
175174
}
176175
}
177-
178-
resourceReleased(key, this._activeResourceCounts);
179176
}
180177
}
181178

test/internal/pool.test.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,40 @@ describe('Pool', () => {
522522
});
523523
});
524524

525+
it('should resolve pending acquisition request when single invalid resource returned', done => {
526+
const key = 'bolt://localhost:7687';
527+
const acquisitionTimeout = 1000;
528+
let counter = 0;
529+
530+
const pool = new Pool(
531+
(url, release) => new Resource(url, counter++, release),
532+
resource => {
533+
},
534+
resourceValidOnlyOnceValidationFunction,
535+
new PoolConfig(1, acquisitionTimeout)
536+
);
537+
538+
pool.acquire(key).then(resource1 => {
539+
expect(resource1.id).toEqual(0);
540+
expect(pool.activeResourceCount(key)).toEqual(1);
541+
542+
// release the resource before the acquisition timeout, it should be treated as invalid
543+
setTimeout(() => {
544+
expectNumberOfAcquisitionRequests(pool, key, 1);
545+
resource1.close();
546+
}, acquisitionTimeout / 2);
547+
548+
pool.acquire(key).then(resource2 => {
549+
expect(resource2.id).toEqual(1);
550+
expectNoPendingAcquisitionRequests(pool);
551+
expect(pool.activeResourceCount(key)).toEqual(1);
552+
done();
553+
}).catch(error => {
554+
done.fail(error);
555+
});
556+
});
557+
});
558+
525559
it('should work fine when invalid resources released and acquisition attempt pending', done => {
526560
const key = 'bolt://localhost:7687';
527561
const acquisitionTimeout = 1000;

0 commit comments

Comments
 (0)