search

brianc / node-pg-pool

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

Implement 'release' event and 'activeCount' property. #125

Open PrimeJunta opened 5 years ago

PrimeJunta commented 5 years ago

Implements feature request from https://github.com/brianc/node-pg-pool/issues/124 .

PrimeJunta commented 5 years ago

Hi @brianc, in case you think the suggestion from the issue I reported is worth addressing, here's an implementation. Thanks for the work you're doing and don't worry, I won't be mad if you decline it.

brianc commented 5 years ago

Hey thanks for the PR! My suggestion would be if you only want to track the active count that we just use an increment/decrement on a this._activeCount = 0 field instead of keeping an array and adding/removing items to it every time we add/remove an active client. What do you think?

PrimeJunta commented 5 years ago

I have no objection. I implemented it that way to maintain consistency with the way you track idle and total clients, and also because I thought there might be a scenario where you'd want to know exactly which clients are checked out -- e.g. if you're debugging a mysterious client leak and track it by flagging each client with something that identifies the process that checked it out. I might be overthinking it however; a simple counter would certainly suffice for the actual scenarios I had in mind.

Would you like me to update the PR or shall you do it yourself?

PrimeJunta commented 5 years ago

I've addressed the change requests @charmander made. If you'd like me to switch from the array to a simple int counter, let me know and I'll make that change also.