brianc
commented
5 years ago Yeah I can't quite review this now until it's rebased on top of master after #109 lands. Initial thoughts:
- Might want to call it
maxCheckoutMillisor something and the event namemaxCheckoutExceededand ifmaxCheckoutMillisis exceeded you can emit a message w/ the client. The thing is...in this situation you probably want to crash your node app because you're doing something wrong and leaking clients...but having a generic event likepool.on('maxCheckoutExceeded', (client) => {}). The idea of 'forcing an unlock' is likely going to just further jack up your application since you'll be using clients which should have been checked in, but aren't...this will make it easier to diagnose though in your app if you're leaking clients - The diff is a bit hard to read, so I'll wait to review the behavior exactly until after 109 is merged.
- I recommend wrapping
pg-poolin a library in your own app that proxies through to it...this allows you to do this kinda thing in your own code: https://node-postgres.com/guides/project-structure
This even will let folks who have scattered access directly to an instance of a pool throughout their application to diagnose things though which is good.
johanneswuerbach
Based on https://github.com/brianc/node-pg-pool/pull/109
Adds a new force unlock timeout, which is by default disabled to stay backwards compatible.
This timeout forcefully ends the client if a client was taken from the pool longer then
forceUnlockTimeoutMillisand the main use-case for this is preventing any kind of pool client leaks, which could render a pool consumers completely unusable.WIP but happy about comments, will add tests as soon as possible.