Node-postgres: Pooled client connections never close in 7.x

[ghafran]

There is a bug with 7.x where the pooled clients never close, no matter what the idleTimeoutMillis is set to.

Even after calling done, the connection remains open indefinitely, essentially ignoring idleTimeoutMillis.

var config = {
    host: database.host,
    port: database.port,
    database: database.database,
    user: database.user,
    password: database.password,
    max: 10,
    idleTimeoutMillis: 10000
};
var pool = new pg.Pool(config);
pool.connect(function (err, client, done) { 
    // use client here
   done(); 
});

Query to see active connections

SELECT count(*), datname FROM pg_stat_activity where state = 'idle'  group by datname;

This works fine with 6.1.0. We have reverted back to 6.1.0 until this is resolved because its causing connection leaks.

Node version: v6.11.2

[brianc]

I'll take a look at this tonight. I really appreciate the concise and easy to run code in the issue. :tada:

[charmander]

I can’t reproduce this with pg 7.1.0, pg-pool 2.0.1, and Node 8.2.1; the connection count drops after 10 seconds as expected. Is the entire script shown there? Also, if you add log: console.log to config, is “remove idle client” ever printed?

[brianc]

Thanks for looking at this @charmander.

@ghafran is there a particular query you're running that's triggering this? in your example you have:

//use client here

Is there a way you can flesh out your example to include some boilerplate query that demonstrates the issue? something like select now() or select pg_sleep(10000) or something?

[ghafran]

@brianc @charmander
Here is the script to fully replicate this issue:

@charmander, remove idle client is never printed, with idleTimeoutMillis: 10000

Run this script with 6.1.0 and then run with 7.1.0, and you will see the difference in connection close timings.

Node version: v6.11.2

var promise = require('bluebird'),
    pg = require('pg'),
    spice = require('pg-spice');

spice.patch(pg);

var config = {
    host: 'dbserver',
    port: 5432,
    database: 'foo',
    user: 'awsome',
    password: 'password',
    max: 10,
    idleTimeoutMillis: 2000
};

var pool = new pg.Pool(config);

function q(query) {
    return new promise((resolve, reject) => {
        pool.connect(function (err, client, done) {
            client.query(query, {}, (err, results) => {
                done();
                if(err) {
                    reject(err);
                } else {
                    resolve(results.rows);
                }
            });
        });
    });
}

// kick off multiple queries
var ps = [];
for(var i = 0; i < 20; i++) {
    ps.push(q(`SELECT * FROM test limit 1`).then((rows) => {
        // console.log(rows.length);
    }));
}
promise.all(ps).then(()=>{
    console.log('queries done');

    // keep checking connections on db
    setInterval(() => {
        q(`SELECT count(*), datname FROM pg_stat_activity where state = 'idle' group by datname`).then((rows) => {
            console.log(rows);
        });
    }, 5000);
});

In 7.1.0, with idleTimeoutMillis = 2000, it takes about 40 seconds to close all the connections. With idleTimeoutMillis = 10000, connections never seem to close.

In 6.1.0, connections close perfectly based on idleTimeoutMillis.

Thanks for the quick responses!

[sehrope]

@ghafran Can you reproduce this after removing pg-spice? I don't think it's directly related to this idle issue but as it monkey patches pg.Client.prototype.query I wouldn't rule it out.

FYI, I wrote that module a while back and would recommend not using it. A better approach is to centralize database access to an app local file (ex: $APP_HOME/config/db.js). Any handling of things like named parameters can go there. If you want something with a similar callback API that also supports named params, but without the monkey patching, check out pg-db.

Separately there's two spots where error handling in your example isn't complete. The err on connect isn't handled and the done() call should pass err as a parameter to evict the connection if it's errant. So something like this (note I haven't tested it, just copy/paste/edit your code):

function q(query) {
    return new promise((resolve, reject) => {
        pool.connect(function (err, client, done) {
            if (err) {
                // Could not create a connection
                reject(err);
                return;
            }
            client.query(query, {}, (err, results) => {
                // If an error occurred executing the query then inform the pool to remove the connection
                done(err);
                if (err) {
                    reject(err);
                } else {
                    resolve(results.rows);
                }
            });
        });
    });
}

Again these may not be directly associated with the timeout issue but would be good to rule them out as well.

[ghafran]

@sehrope Thanks for the note. The pg-spice is not the problem, I did remove this from my test and still got the same issue. Error handling is a good practice, even with that same problem.

[brianc]

@ghafran - @charmander fixed your issue. Please try with [email protected] and lemme know if it's fixed. Sorry for the bug! 🙇

[ghafran]

npm install pg-pool --save fixed the issue!

Upgrading to 7.1.0.

You guys rock!

[brianc]

👏