diff --git a/README.md b/README.md index 4c577d2a..a02a86e4 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,7 @@ An optional object/dictionary with the any of the following properties: - `numTestsPerRun`: Number of resources to check each eviction run. Default: 3. - `softIdleTimeoutMillis`: amount of time an object may sit idle in the pool before it is eligible for eviction by the idle object evictor (if any), with the extra condition that at least "min idle" object instances remain in the pool. Default -1 (nothing can get evicted) - `idleTimeoutMillis`: the minimum amount of time that an object may sit idle in the pool before it is eligible for eviction due to idle time. Supercedes `softIdleTimeoutMillis` Default: 30000 +- `maxTries`: maximum number of consequent failures before pool stops trying to create resource and rejects resource request. Default: 0 (tries forever until resource is created). - `Promise`: Promise lib, a Promises/A+ implementation that the pool should use. Defaults to whatever `global.Promise` is (usually native promises). ### pool.acquire diff --git a/lib/Pool.js b/lib/Pool.js index 70bfd5a3..d1b59441 100644 --- a/lib/Pool.js +++ b/lib/Pool.js @@ -3,6 +3,7 @@ const EventEmitter = require("events").EventEmitter; const factoryValidator = require("./factoryValidator"); +const errors = require("./errors"); const PoolOptions = require("./PoolOptions"); const ResourceRequest = require("./ResourceRequest"); const ResourceLoan = require("./ResourceLoan"); @@ -123,6 +124,12 @@ class Pool extends EventEmitter { */ this._scheduledEviction = null; + /** + * counter of consequent failures to create resource + * @type {Number} + */ + this._failedTries = 0; + // create initial resources (if factory.min > 0) if (this._config.autostart === true) { this.start(); @@ -215,6 +222,15 @@ class Pool extends EventEmitter { return; } + // Try counter exceeded - reject resource request to next waiting client + if ( + this._config.maxTries > 0 && + this._failedTries >= this._config.maxTries + ) { + this._dispatchPooledResourceToNextWaitingClient(); + return; + } + const resourceShortfall = numWaitingClients - this._potentiallyAllocableResourceCount; @@ -267,14 +283,32 @@ class Pool extends EventEmitter { ) { // While we were away either all the waiting clients timed out // or were somehow fulfilled. put our pooledResource back. - this._addPooledResourceToAvailableObjects(pooledResource); + if (pooledResource) { + // While we were away either all the waiting clients timed out + // or were somehow fulfilled. put our pooledResource back. + this._addPooledResourceToAvailableObjects(pooledResource); + } // TODO: do need to trigger anything before we leave? return false; } - const loan = new ResourceLoan(pooledResource, this._Promise); - this._resourceLoans.set(pooledResource.obj, loan); - pooledResource.allocate(); - clientResourceRequest.resolve(pooledResource.obj); + if (pooledResource) { + const loan = new ResourceLoan(pooledResource, this._Promise); + this._resourceLoans.set(pooledResource.obj, loan); + pooledResource.allocate(); + clientResourceRequest.resolve(pooledResource.obj); + } else { + let errorMessage = "Failed to create resource"; + if ( + this._config.maxTries > 0 && + this._failedTries >= this._config.maxTries + ) { + errorMessage = + "Failed to create resource " + this._failedTries + " times in a row"; + } + clientResourceRequest.reject( + new errors.ResourceCreationError(errorMessage) + ); + } return true; } @@ -304,16 +338,26 @@ class Pool extends EventEmitter { * @private */ _createResource() { + // Do not attempt to create resource if reached maximum number of consequent failures + if ( + this._config.maxTries > 0 && + this._failedTries >= this._config.maxTries + ) { + return; + } // An attempt to create a resource const factoryPromise = this._factory.create(); const wrappedFactoryPromise = this._Promise.resolve(factoryPromise); this._trackOperation(wrappedFactoryPromise, this._factoryCreateOperations) .then(resource => { + // Resource created successfully - reset fail counter + this._failedTries = 0; this._handleNewResource(resource); return null; }) .catch(reason => { + this._failedTries++; this.emit(FACTORY_CREATE_ERROR, reason); this._dispense(); }); @@ -435,6 +479,10 @@ class Pool extends EventEmitter { ); } + // Reset fail counter - maybe resource will be created this time + // (for example network is restored and connection can be made) + this._failedTries = 0; + // TODO: should we defer this check till after this event loop incase "the situation" changes in the meantime if ( this._config.maxWaitingClients !== undefined && diff --git a/lib/PoolOptions.js b/lib/PoolOptions.js index 498c0e34..c4f8a88d 100644 --- a/lib/PoolOptions.js +++ b/lib/PoolOptions.js @@ -42,6 +42,9 @@ class PoolOptions { * @param {Number} [opts.idleTimeoutMillis=30000] * the minimum amount of time that an object may sit idle in the pool before it is eligible for eviction * due to idle time. Supercedes "softIdleTimeoutMillis" Default: 30000 + * @param {Number} opts.maxTries + * maximum number of consequent failures before pool stops trying to create resource + * and rejects resource request. Default: 0 (tries forever until resource is created). * @param {typeof Promise} [opts.Promise=Promise] * What promise implementation should the pool use, defaults to native promises. */ @@ -81,9 +84,11 @@ class PoolOptions { this.max = parseInt(opts.max, 10); // @ts-ignore this.min = parseInt(opts.min, 10); + this.maxTries = parseInt(opts.maxTries, 10); this.max = Math.max(isNaN(this.max) ? 1 : this.max, 1); this.min = Math.min(isNaN(this.min) ? 0 : this.min, this.max); + this.maxTries = isNaN(this.maxTries) ? 0 : this.maxTries; this.evictionRunIntervalMillis = opts.evictionRunIntervalMillis || poolDefaults.evictionRunIntervalMillis; diff --git a/lib/errors.js b/lib/errors.js index b02d822f..31650a1f 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -20,8 +20,15 @@ class TimeoutError extends ExtendableError { super(m); } } + +class ResourceCreationError extends ExtendableError { + constructor(m) { + super(m); + } +} /* eslint-enable no-useless-constructor */ module.exports = { - TimeoutError: TimeoutError + TimeoutError: TimeoutError, + ResourceCreationError: ResourceCreationError }; diff --git a/test/generic-pool-maxtries-test.js b/test/generic-pool-maxtries-test.js new file mode 100644 index 00000000..e31ebcaa --- /dev/null +++ b/test/generic-pool-maxtries-test.js @@ -0,0 +1,76 @@ +"use strict"; + +const tap = require("tap"); +const createPool = require("../").createPool; + +tap.test("maxTries handles acquire exhausted calls", function(t) { + let resourceCreationAttempts = 0; + const factory = { + create: function() { + resourceCreationAttempts++; + if (resourceCreationAttempts < 5) { + return Promise.reject(new Error("Create Error")); + } + return Promise.resolve({}); + }, + destroy: function() { + return Promise.resolve(); + } + }; + const config = { + maxTries: 3 + }; + + const pool = createPool(factory, config); + + pool + .acquire() + .then(function(resource) { + t.fail("wooops"); + }) + .catch(function(err) { + t.match(err, /ResourceCreationError: Failed to create resource/); + return pool.drain(); + }) + .then(function() { + return pool.clear(); + }) + .then(function() {}) + .then(t.end) + .catch(t.error); +}); + +tap.test("maxTries handles acquire non exhausted calls", function(t) { + const myResource = {}; + let resourceCreationAttempts = 0; + const factory = { + create: function() { + resourceCreationAttempts++; + if (resourceCreationAttempts < 2) { + return Promise.reject(new Error("Create Error")); + } + return Promise.resolve(myResource); + }, + destroy: function() { + return Promise.resolve(); + } + }; + const config = { + maxTries: 3 + }; + + const pool = createPool(factory, config); + + pool + .acquire() + .then(function(resource) { + t.equal(resource, myResource); + pool.release(resource); + return pool.drain(); + }) + .then(function() { + return pool.clear(); + }) + .then(t.end) + .catch(t.error); +});