Allow to use a pool of connection to PostgreSQL #97

Merged
merged 1 commit into from Oct 4, 2017

Conversation

Projects
None yet
3 participants
@ldidry
Contributor

ldidry commented May 18, 2017

UeberDB currently use only one connection to PostgreSQL, which is insufficient for our high-traffic Etherpad instances.

This commit include a bump of pg dependency version and a new db connector, postgrespool.

It has been tested in production on several instances, one of it hosting more than 44,000 pads and heavily used.

@ldidry

This comment has been minimized.

Show comment
Hide comment
@ldidry

ldidry Oct 2, 2017

Contributor

Ping !

Contributor

ldidry commented Oct 2, 2017

Ping !

@ldidry ldidry referenced this pull request Oct 2, 2017

Open

Postgres now work #98

@JohnMcLear JohnMcLear merged commit 7b268b8 into Pita:master Oct 4, 2017

@muxator

This comment has been minimized.

Show comment
Hide comment
@muxator

muxator Oct 4, 2017

Contributor

Hi,

using the connection pool is awesome.
The two drivers are really similar: since we already bumped the pg version, shouldn't we simply port the changes in the old driver and remove the duplicated code?

$ diff --ignore-space-change  --unified postgres_db.js postgrespool_db.js
--- postgres_db.js	2017-10-05 00:45:45.977646205 +0200
+++ postgrespool_db.js	2017-10-05 00:45:45.977646205 +0200
@@ -25,8 +25,12 @@
   this.settings.writeInterval = 100;
   this.settings.json = true;
 
-  this.db = new pg.Client(this.settings);
-  this.db.connect();
+  // Pool specific defaults
+  this.settings.max = this.settings.max || 20;
+  this.settings.min = this.settings.min || 4;
+  this.settings.idleTimeoutMillis = this.settings.idleTimeoutMillis || 1000;
+
+  this.db = new pg.Pool(this.settings);
 
 }
 
@@ -179,10 +185,5 @@
 
 exports.database.prototype.close = function(callback)
 {
-  var _this = this;
-  this.db.on('drain', function() {
-  	_this.db.end.bind(_this.db);
-  	callback(null);
-  });
-  
+  this.db.end()
 }
Contributor

muxator commented Oct 4, 2017

Hi,

using the connection pool is awesome.
The two drivers are really similar: since we already bumped the pg version, shouldn't we simply port the changes in the old driver and remove the duplicated code?

$ diff --ignore-space-change  --unified postgres_db.js postgrespool_db.js
--- postgres_db.js	2017-10-05 00:45:45.977646205 +0200
+++ postgrespool_db.js	2017-10-05 00:45:45.977646205 +0200
@@ -25,8 +25,12 @@
   this.settings.writeInterval = 100;
   this.settings.json = true;
 
-  this.db = new pg.Client(this.settings);
-  this.db.connect();
+  // Pool specific defaults
+  this.settings.max = this.settings.max || 20;
+  this.settings.min = this.settings.min || 4;
+  this.settings.idleTimeoutMillis = this.settings.idleTimeoutMillis || 1000;
+
+  this.db = new pg.Pool(this.settings);
 
 }
 
@@ -179,10 +185,5 @@
 
 exports.database.prototype.close = function(callback)
 {
-  var _this = this;
-  this.db.on('drain', function() {
-  	_this.db.end.bind(_this.db);
-  	callback(null);
-  });
-  
+  this.db.end()
 }

@ldidry ldidry referenced this pull request Mar 1, 2018

Closed

Bump pg dependency #94

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment