Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added option.maxTries #184

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
58 changes: 53 additions & 5 deletions lib/Pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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();
});
Expand Down Expand Up @@ -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 &&
Expand Down
5 changes: 5 additions & 0 deletions lib/PoolOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 8 additions & 1 deletion lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
76 changes: 76 additions & 0 deletions test/generic-pool-maxtries-test.js
Original file line number Diff line number Diff line change
@@ -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);
});