brianc / node-pg-pool

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

Support async_hooks #108

Open chrisdickinson opened 5 years ago

chrisdickinson commented 5 years ago

(To preface: thank you for providing & maintaining this package! We make extensive use of it within npm. I'm proposing support for a feature here but I want to stress that I don't want to add more to your to-do list. If you're amenable I'd be happy to implement it myself, with help from @mcollina.)

For context: We've been using domains with node-postgres in order to associate database connections with requests (in a form of continuation local storage.) Domains have been deprecated, and as of Node 10 they appear to incur a substantial performance hit. The behavior we actually rely on from domains — the behavior of continuation context association — has been absorbed into a new API, async_hooks, meant to provide this functionality to APM vendors.

The problem is that async_hooks and pooling libraries interact in undesirable ways: because an empty pool creates a connection for the first requester, the connection is associated with the requester's async context. When the connection is returned to the pool and requested again, it remains associated with the first requester's async context. My understanding (& @mcollina can correct me where I'm wrong) is that it's possible to wrap pool instances with AsyncResource, in order to associate outgoing connections with their requester's context.

My proposal is to undertake the necessary work to integrate AsyncResource, if you'd be amenable to it. I believe this would help us (& APM companies) out. With that, I'd like to hand over the floor to @mcollina who can correct me where I'm wrong / expand on the proposed changes that would need to be made.

mcollina commented 5 years ago

I'm happy to review such a change and test the integration.