Fails to reconnect after invoking end()

[vitaly-t]

We call end() on the pool in order to shut down all the connections within the pool.

However, doing so prevents us from being able to use the object any longer.

Any attempt of calling .connect after .end() was called once results in the following error:

pool is draining and cannot accept work

Below is a complete test application:

'use strict';

var pg = require('pg');

var config = {
    database: 'newone',
    port: 5433,
    user: 'postgres'
};

var pool = new pg.Pool(config);

function test1() {
    pool.connect((err, client, done) => {
        if (err) {
            console.log('Error Connecting:', err);
            return;
        }
        client.query('select count(*) from users', (err, data) => {
            if (err) {
                console.log('Query Error:', err);
            } else {
                console.log('DATA:', data.rows);
            }
            done();
            pool.end();
            test2();
        });
    });
}

function test2() {
    pool.connect((err, client, done) => {
        if (err) {
            console.log('Error Connecting:', err);
            return;
        }
        client.query('select count(*) from users', (err, data) => {
            if (err) {
                console.log('Query Error:', err);
            } else {
                console.log('DATA:', data.rows);
            }
            done();
            pool.end();
        });
    });
}

test1();

test1 ends successfully, but in test2 it always throws that error on this line:

console.log('Error Connecting:', err);
//=> pool is draining and cannot accept work

[brianc]

Yeah - once you call end() on the pool it's over: it's being disposed of. You'll need to create a new pool. That's behavior as designed.

[vitaly-t]

But why? What is the reason for that?

[brianc]

That's how generic pool works: https://github.com/coopernurse/node-pool#draining

[vitaly-t]

I don't think it matters how that pool works though.

You encapsulate that pool in your library, which means you can set it to null within the end function, and then create a new one within the connect function, if required.

This would make it consistent with how connections used to work, and still work via pg.connect, that we can call .end() and then call connect after that successfully.

P.S. It mostly inconveniences automated tests, but sometimes also real applications, if you do not have direct access to the object that holds the pool, and want to execute queries indirectly.

[vitaly-t]

or....maybe it is for the better...? I'm still debating over the cons and pros of it...

[vitaly-t]

@brianc If we agree that this is ok for the pool to shut down itself permanently, then we definitely need a way to determine when this has happened.

When we share a Pool object we need to be able in some situations to ask - Was this pool terminated / ended / finished / capoot?

Something like this:

if(pool.isTerminated) {
    // do what needs to be done
}

When using the pool inside another high-level library it requires some intelligence, in order to provide good diagnostics, that's what I'm working on now.

[charmander]

Why would you destroy the pool if you still wanted to use it?

[vitaly-t]

@charmander as per an earlier example, a Pool object is exposed by the library, to be shared with other third-party libraries or modules, in order to avoid creation of an additional pool.

A third-party library / module may incorrectly end the pool, thus breaking all the other modules that use it.

As I wrote earlier, it is probably wise to keep it she Pool object destroyed, but we at least need a way to determine that, so it can be diagnosed easily, something like pool.isTerminated, see the example above.

[charmander]

but we at least need a way to determine that, so it can be diagnosed easily

Maybe the message saying the pool is draining? That’s what .end() does. You could submit a wording pull request on generic-pool.

[vitaly-t]

Maybe the message saying the pool is draining

We don't need any message, just an indication/flag, so any library can provide its own message.

You could submit a wording pull request on generic-pool.

Addition of such a flag would be trivial. I will do it now 😉

[vitaly-t]

I've just found out that we can verify it through the generic-pool library:

var p = new pg.Pool(connection);
var isTerminated = p.pool._draining;

though I'm not sure if it is a very good one, to rely on an internal module for this. It can change in that module, that in turn will break our module that uses node-postgres. Probably best is still to add that isTerminated property.

[vitaly-t]

Added a PR: #59