Client pooling

[Voles]

First of all, thank you for your work!

I’m using FortuneJS with the PostgreSQL adapter and Ember Data.
Now, when the server has been up for a while, the following error occurs:

Unhandled rejectionError: Connection terminated

It comes from pg/lib/client.js.

My guess is that this error occurs because connections aren't properly closed, or there are too much open connections. This error occurred in a test-environment with only 2 users.

After looking through the code I found out that there was no client pooling configured.
Is there a reason client pooling isn’t implemented? I’m willing to set up a PR if you think it would be a good addition.

More details: https://github.com/brianc/node-postgres#client-pooling

Thanks again!

[gr0uch]

Oh, that's new.

https://github.com/brianc/node-pg-pool
This was released a month ago, and I wasn't aware of that.

This should be an automatic upgrade, nothing new needed from the client side.

A PR would be greatly welcome.

[Voles]

Thanks, I'll look into it.

I still have some reservations:

• while fortune-postgres uses a connection string, the configuration object for pooling requires the DB user, password and name to be separate properties
• for what I see there should be no API changes for fortune-postgres, do you think we should always enable pooling?

[gr0uch]

It seems the way to do this is to re-use https://github.com/brianc/node-postgres/blob/master/lib/connection-parameters.js

Also added a PR for connection parameters in the Pool constructor: brianc/node-postgres#1068

[Voles]

@0x8890 I've looked into the source of node-postgres. When calling pg's connect() method, it seems that it's automatically setting up a pool. See https://github.com/brianc/node-postgres/blob/master/lib/index.js#L69

However, that feels odd, because then why would they have a complete section covering Client pooling.

I'm not yet sure how to implement this.

[gr0uch]

from what I understood, using pg.connect gives you client pooling, so I actually thought that this adapter uses pooling already. What would change I think is explicitly creating a pool in the adapter, but first brianc/node-postgres#1068 needs to be merged.

[Voles]

@0x8890 I think you're right. However, the disadvantage of using a connection string is that the following configuration options are not available, which means the default values will be used:

• idleTimeoutMillis
• poolLog
• poolSize

My original issue about the server crashing because clients aren't handled well is probably invalid. If node-pg creates a pool by default, handling clients should not be an issue for my application.