coopernurse / node-pool

Generic resource pooling for node.js
2.38k stars 259 forks source link

Question: What is the expected behavior when you try to acquire a connection exceeding the max number? #130

Closed subeeshcbabu-zz closed 8 years ago

subeeshcbabu-zz commented 8 years ago

What is the expected behavior?.

var Pool = require('generic-pool').Pool;

var mypool = Pool({
    name     : 'mypool',
    create   : function (callback) {
        callback(null, {
            id: Math.random(),
            details: 'This is a connection'
        });
    },
    destroy  : function (connection) {
        console.log("Destroyed");
    },
    max      : 1,
    min      : 0,
    idleTimeoutMillis : 5000
});

console.log("acquiring connection");

mypool.acquire(function(error, connection){

    if (error) {
        console.error(error);
    }
    console.log('Got connection', connection);

    mypool.acquire(function(error, connection2){

        if (error) {
            console.error(error);
        }
        console.log('Got connection2', connection2);
    });
});
sandfox commented 8 years ago

Hi @subeeshcbabu

Sorry for the slow response. Up to and including current published version 2.4.x the behaviour is to allow infinite number of queued requests. This means that calls to acquire will always be accepted and queued up for allocation. This can lead to deadlocking in some situations if your application never returns resources to the pool

There are PRs in progress #125 and #127 to allow acquire calls to fail if the pool is full, and/or timeout acquire calls if they aren't fulfilled in a given time frame, and I intend to add a feature to allow configuring the max number of "waiting" acquire calls.

Let me know if this helps or you have any more questions.

James

subeeshcbabu-zz commented 8 years ago

This pretty much answers, what I was looking for. Thx. In fact, I did notice those PRs after raising this question here. My +1 on the timeout implementation and maxWaitingClients options. Looking forward to those features getting merged.

For now, I have implemented the acquireTimeout feature myself, for the use case that I am solving.