Remove internal pool (#1049)

* Initial work on removing internal pool

* Port backwards-compabible properties

* Cleanup test execution & makefile cruft

* Attempt to fix flakey error test

Makefile

@@ -13,7 +13,6 @@ all:
 	npm install
 
 help:
-	@echo "make prepare-test-db [connectionString=postgres://<your connection string>]"
 	@echo "make test-all [connectionString=postgres://<your connection string>]"
 
 test: test-unit
@@ -32,11 +31,7 @@ test-unit:
 
 test-connection:
 	@echo "***Testing connection***"
-	@node script/test-connection.js $(params)
-
-test-connection-binary:
-	@echo "***Testing binary connection***"
-	@node script/test-connection.js $(params) binary
+	@node script/create-test-tables.js $(params)
 
 test-missing-native:
 	@echo "***Testing optional native install***"
@@ -47,7 +42,7 @@ test-missing-native:
 node_modules/pg-native/index.js:
 	@npm i pg-native
 
-test-native: node_modules/pg-native/index.js
+test-native: node_modules/pg-native/index.js test-connection
 	@echo "***Testing native bindings***"
 	@find test/native -name "*-tests.js" | $(node-command)
 	@find test/integration -name "*-tests.js" | $(node-command) native
@@ -56,13 +51,12 @@ test-integration: test-connection
 	@echo "***Testing Pure Javascript***"
 	@find test/integration -name "*-tests.js" | $(node-command)
 
-test-binary: test-connection-binary
+test-binary: test-connection
 	@echo "***Testing Pure Javascript (binary)***"
 	@find test/integration -name "*-tests.js" | $(node-command) binary
 
-prepare-test-db:
-	@echo "***Preparing the database for tests***"
-	@find script/create-test-tables.js  | $(node-command)
+test-pool:
+	@find test/integration/connection-pool -name "*.js" | $(node-command) binary
 
 jshint:
 	@echo "***Starting jshint***"

README.md

@@ -19,13 +19,21 @@ $ npm install pg
 Generally you will access the PostgreSQL server through a pool of clients.  A client takes a non-trivial amount of time to establish a new connection. A client also consumes a non-trivial amount of resources on the PostgreSQL server - not something you want to do on every http request. Good news: node-postgres ships with built in client pooling.
 
 ```javascript
-var pg = require('pg');
-var conString = "postgres://username:password@localhost/database";
+var Pool = require('pg').Pool;
+
+var config = {
+  user: 'foo', //env var: PGUSER
+  database: 'my_db', //env var: PGDATABASE
+  password: 'secret', //env var: PGPASSWORD
+  port: 5432 //env var: PGPORT
+};
+
+var pool = new Pool(config);
 
 //this initializes a connection pool
 //it will keep idle connections open for a (configurable) 30 seconds
 //and set a limit of 10 (also configurable)
-pg.connect(conString, function(err, client, done) {
+pool.connect(function(err, client, done) {
   if(err) {
     return console.error('error fetching client from pool', err);
   }
@@ -42,6 +50,8 @@ pg.connect(conString, function(err, client, done) {
 });
 ```
 
+node-postgres uses [pg-pool](https://github.com/brianc/node-pg-pool.git) to manage pooling and only provides a very thin layer on top.  It's highly recommend you read the documentation for [pg-pool](https://github.com/brianc/node-pg-pool.git)
+
 [Check this out for the get up and running quickly example](https://github.com/brianc/node-postgres/wiki/Example)
 
 ### Client instance
@@ -85,7 +95,7 @@ node-postgres contains a pure JavaScript protocol implementation which is quite
 
 To use the native bindings, first install [pg-native](https://github.com/brianc/node-pg-native.git).  Once pg-native is installed, simply replace `require('pg')` with `require('pg').native`.
 
-node-postgres abstracts over the pg-native module to provide exactly the same interface as the pure JavaScript version. __No other code changes are required__.  If you find yourself having to change code other than the require statement when switching from `require('pg')` to `require('pg').native` please report an issue.
+node-postgres abstracts over the pg-native module to provide exactly the same interface as the pure JavaScript version. Care has been taken to keep the number of api differences between the two modules to a minimum; however, it is recommend you use either the pure JavaScript or native bindings in both development and production and don't mix & match them in the same process - it can get confusing!
 
 ## Features
 

lib/index.js

@@ -2,15 +2,17 @@ var EventEmitter = require('events').EventEmitter;
 var util = require('util');
 var Client = require('./client');
 var defaults =  require('./defaults');
-var pool = require('./pool');
 var Connection = require('./connection');
+var ConnectionParameters = require('./connection-parameters');
+var Pool = require('pg-pool');
 
 var PG = function(clientConstructor) {
   EventEmitter.call(this);
   this.defaults = defaults;
   this.Client = clientConstructor;
   this.Query = this.Client.Query;
-  this.pools = pool(clientConstructor);
+  this.Pool = Pool;
+  this.pools = [];
   this.Connection = Connection;
   this.types = require('pg-types');
 };
@@ -19,16 +21,16 @@ util.inherits(PG, EventEmitter);
 
 PG.prototype.end = function() {
   var self = this;
-  var keys = Object.keys(self.pools.all);
+  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.all[key];
-      delete self.pools.all[key];
-      pool.drain(function() {
-        pool.destroyAllNow(function() {
+      var pool = self.pools[key];
+      delete self.pools[key];
+      pool.pool.drain(function() {
+        pool.pool.destroyAllNow(function() {
           count--;
           if(count === 0) {
             self.emit('end');
@@ -39,17 +41,31 @@ PG.prototype.end = function() {
   }
 };
 
-
 PG.prototype.connect = function(config, callback) {
   if(typeof config == "function") {
     callback = config;
     config = null;
   }
-  var pool = this.pools.getOrCreate(config);
+  var poolName = JSON.stringify(config || {});
+  if (typeof config == 'string') {
+    config = new ConnectionParameters(config);
+  }
+
+  config = config || {};
+
+  //for backwards compatibility
+  config.max = config.max || config.poolSize || defaults.poolSize;
+  config.idleTimeoutMillis = config.idleTimeoutMillis || config.poolIdleTimeout || defaults.poolIdleTimeout;
+  config.log = config.log || config.poolLog || defaults.poolLog;
+
+  this.pools[poolName] = this.pools[poolName] || new Pool(config, this.Client);
+  var pool = this.pools[poolName];
   pool.connect(callback);
   if(!pool.listeners('error').length) {
     //propagate errors up to pg object
-    pool.on('error', this.emit.bind(this, 'error'));
+    pool.on('error', function(e) {
+      this.emit('error', e, e.client);
+    }.bind(this));
   }
 };
 

package.json

@@ -19,17 +19,19 @@
   "main": "./lib",
   "dependencies": {
     "buffer-writer": "1.0.1",
-    "generic-pool": "2.4.2",
     "packet-reader": "0.2.0",
     "pg-connection-string": "0.1.3",
+    "pg-pool": "1.*",
     "pg-types": "1.*",
     "pgpass": "0.0.6",
     "semver": "4.3.2"
   },
   "devDependencies": {
     "async": "0.9.0",
     "jshint": "2.5.2",
-    "pg-copy-streams": "0.3.0"
+    "lodash": "4.13.1",
+    "pg-copy-streams": "0.3.0",
+    "promise-polyfill": "5.2.1"
   },
   "minNativeVersion": "1.7.0",
   "scripts": {

script/create-test-tables.js

@@ -38,23 +38,17 @@ var con = new pg.Client({
   database: args.database
 });
 con.connect();
-if(args.down) {
-  console.log("Dropping table 'person'")
-  var query = con.query("drop table if exists person");
-  query.on('end', function() {
-    console.log("Dropped!");
-    con.end();
-  });
-} else {
-  console.log("Creating table 'person'");
-  con.query("create table person(id serial, name varchar(10), age integer)").on('end', function(){
-    console.log("Created!");
-    console.log("Filling it with people");
-  });;
-  people.map(function(person) {
-    return con.query("insert into person(name, age) values('"+person.name + "', '" + person.age + "')");
-  }).pop().on('end', function(){
-    console.log("Inserted 26 people");
-    con.end();
-  });
-}
+var query = con.query("drop table if exists person");
+query.on('end', function() {
+  console.log("Dropped table 'person'")
+});
+con.query("create table person(id serial, name varchar(10), age integer)").on('end', function(){
+  console.log("Created table person");
+  console.log("Filling it with people");
+});
+people.map(function(person) {
+  return con.query("insert into person(name, age) values('"+person.name + "', '" + person.age + "')");
+}).pop().on('end', function(){
+  console.log("Inserted 26 people");
+  con.end();
+});