Add PG.prototype.disconnect method?

[AlJohri]

Hi,

It seems that connection pool objects have an independent end function (Pool.prototype.end).

Pool.prototype.end = function (cb) {
  this.log('draining pool')
  return this._promise(cb, function (resolve, reject) {
    this.pool.drain(function () {
      this.log('pool drained, calling destroy all now')
      this.pool.destroyAllNow(resolve)
    }.bind(this))
  }.bind(this))
}

Currently the PG.prototype.end function does not call each pool's end function and instead manually closes the connection pool by calling the lower level drain and destroyAllNow functions.

PG.prototype.end = function() {
  var self = this;
  var keys = Object.keys(this._pools);
  var count = keys.length;
  if(count === 0) {
    self.emit('end');
  } else {
    keys.forEach(function(key) {
      var pool = self._pools[key];
      delete self._pools[key];
      pool.pool.drain(function() {
        pool.pool.destroyAllNow(function() {
          count--;
          if(count === 0) {
            self.emit('end');
          }
        });
      });
    });
  }
};

My requests:

1. Can we change PG.prototype.end to use Pool.prototype.end? This would by more DRY.

2. Can we add a PG.prototype.disconnect function? It would have the same signature as PG.prototype.connect.

PG.prototype.connect = function(config, callback) {
  ...
  var poolName = JSON.stringify(config || {});
  ...
  this._pools[poolName] = this._pools[poolName] || new this.Pool(config);
  var pool = this._pools[poolName];
  ...
  return pool.connect(callback);
}

Essentially, there is a public API to add a connection pool to _pools and I was looking for a public api to remove a particular connection pool.

The disconnect function would similarly JSON.stringify the config object to find the key and then remove it from this._pools.

Thanks,

Al

[charmander]

It would be better to create your own pg.Pool instance(s) as the example in the README shows: https://github.com/brianc/node-postgres#client-pooling

pg.connect shouldn’t really be used.

[vitaly-t]

pg.connect shouldn’t really be used.

pg.connect is the only one to be used with node-postgres up to version 5.1, and it was doing a better job than the change introduced later: it was simpler, and it could always guarantee to restore lost connections, is where the new pool fails.

[charmander]

it could always guarantee to restore lost connections, is where the new pool fails

Could you give an example of this, please? Sounds like a potential bug report.

[vitaly-t]

Could you give an example of this, please? Sounds like a potential bug report.

• Handling connection timeouts
• Database disconnect handling

[rpedela]

pg.connect shouldn’t really be used.

@charmander That is bad advice. If someone is using an external pooler (e.g. pgbouncer) or none at all, there is no reason to use the built-in pooling.

[charmander]

@rpedela You’re confusing pg.connect with pg.Client.prototype.connect. pg.connect uses an implicit global pool.

[rpedela]

Ah, you are right. Sorry.

[charmander]

pg.connect has been removed in pg 7.