brianc / node-pg-pool

A connection pool for node-postgres
MIT License
180 stars 64 forks source link

Changing connection string in Pool's life cycle #100

Open tihomirmagdic opened 6 years ago

tihomirmagdic commented 6 years ago

I'm interested in how to use Pool with different users (different connection string). My postgres users have different roles and privileges. According to documentation, I can pass properties to the Pool and they are passed unchanged to both the node-postgres Client constructor and the node-pool constructor. But if there is any idle client, the Pool doesn't check if pool's connection parameters (e.g. user, host, port...) are changed in the meantime or not, Pool just returns that (last) idle client.

If a new connection string is provided and there’s no client in the Pool with that connection string, a new client must be created regardless of the existence of an idle client in the Pool with another connection string.

Maybe the simplest solution is a pool of pools - associated array (an object with properties where each property name is a connection string, and property value is Pool instance). It seems a little heavy. Any other idea for getting a client from the pool considering the connection string?

sehrope commented 6 years ago

Maybe the simplest solution is a pool of pools - associated array (an object with properties where each property name is a connection string, and property value is Pool instance). It seems a little heavy. Any other idea for getting a client from the pool considering the connection string?

Older versions of node-postgres did exactly this internally. There was an implicit pool created based upon the connection parameters created on first use. It was removed a couple major versions back because hiding it within the driver was a bad idea (in particular due to lack of insight into resource management).

Doing it yourself is fine though. Create a getPool(connStr) function to retrieve the cached pool or create a new one if it doesn't exist. The connStr can be your cache key.

Maybe the simplest solution is a pool of pools - associated array (an object with properties where each property name is a connection string, and property value is Pool instance). It seems a little heavy. Any other idea for getting a client from the pool considering the connection string?

Either that or simply create a new connection each time. If the connections are only being used for one request then it wouldn't be in the cache and the pool wouldn't help.

With a lot of distinct connection strings you also have the issue of creating and never destroying a lot of distinct pools. The connections in each pool will eventually be reaped (by default after 30 seconds) but the pools themselves would never be removed unless you code that in yourself. Individual connections that are created/destroyed per request may lead to better long term resource usage in this situation.

Another other option is to have a role that can assume the other sub-roles via "SET ROLE ..." (https://www.postgresql.org/docs/current/static/sql-set-role.html). You could then have a single pool of the parent role and wrap the per-user query executions with statements that align the connection for the user's role. See here for an example of this: https://github.com/brianc/node-postgres/issues/1598#issuecomment-374919852

That'd only work if it's the same cluster and database. Also, be aware of the security ramifications of that setup. If you're not in total control of the SQL being executed it'd be possible to break out of the role restriction by executing another SET ROLE ....

tihomirmagdic commented 6 years ago

SET ROLE doesn't work in the case of multiple db on one or multiple hosts.

In Pool, connect function pops last idle client (if any exists) or creates a new one in the _pendingQueue. Prior to creating a new one, it should check if any idle client has the same connection params and returns that one instead of returning always the last one. This eliminates a lot of distinct empty pools. Another solution maybe could be to call Pool.end() on timeout of the last client in the pool.

I was wondering which solutions have proved to be simple for such a problem.

charmander commented 6 years ago

You might want to write a custom pool implementation based on generic-pool (for example).