2.0

[brianc]

This contains a massive number of internal changes. However, the external API remains very close to the same.

The breaking changes are:

• Drop support for node < 4.x. closes #56
• No longer use node-pool as a dependency so any configuration options you previously passed to node-pool will not work anymore except for max and idleTimeoutMillis
• Cannot call pool constructor without new keyword

I attempted to upgrade to node-pool@3.x, but it weirdly no longer supports returning an error when a client fails to connect. Instead it tries to reconnect the client in a tight loop and makes the error non-actionable. I was unable to find a good way to hook into this behavior and override it. Furthermore it has a lot of additional bells and whistles for priority queuing, min levels, auto-connection, and other behavior pg and pg-pool have never used. Initially I was distraught at the thought of implementing the pooling behavior here, but it turns out it wasn't very much code & all the tests still pass!

After this I spent some time going through and writing tests for many of the open issues to verify they no longer happen. I also added support for the long requested connectionTimeoutMillis - now supplying connectionTimeoutMillis as a millisecond value will cause the pool to return an error on your connect call if a connection could not be established within the configured time. By default there is no timeout.

Known fixes:

• Destroyed workers are always recreated without the previous workarounds for node-pool
• pg>=5.0 is a peerDependency. closes #45
• added .ending property which will allow you to inspect when the pool is in the process of shutting down. closes #58
• ensure idleTimeoutMillis of 0 disables idle timeout. closes #64

[brianc]

Thank you so much for the feedback and review! I'm not quite done w/ this branch yet - code's still a bit dirty...but have quite a bit of client work in the next few days so I'll take me a bit of time to get to this. But know I'm grateful & I'll be back within a day or two! :dancer:

[brianc]

I did notice that promise support is still a bit second class. In that -- promisified apis would be able to perform much better than the scheme currently used.

@calebboyd - using callbacks internally in this module is so we can support pg>=5.x instead of pg@7 which is when all the APIs for pg are fully promisified. What do you mean by that exactly - could you elaborate?

[calebboyd]

Mostly I think that it would be best to stick with one API or the other. My reasoning is that: accepting a custom Promise at this level would cut into performance.

For example, Bluebird.promisify, will rewrite the function (emit new code) in order to maintain low overhead when allocating new Promises -- and then uses an internal implementation. Node's require('util').promisify will use a native deferred api. Each of which would behave much faster than creating the new Promise and deferred mechanism being used. As a plus, plain callbacks for waiters in this internal queue will always be the quicker solution of the two.

In general sticking with callbacks here will allow people who use promises (I'm one) to do so with the lowest overhead, which is a plus for the library that proxies queries.

[calebboyd]

I guess that wasn't super clear. I don't have anything that would stop me from using this (I could promisify it anyways to avoid the issue). But Maybe the removal of this.Promise such that promisify is encouraged instead. As that is generally blessed now (in node core)

[brianc]

I'll take a look at hard look at the code once I finish it to ensure it's not doing unnecessary work wrt allocating promises. It should only allocate a promise if you don't supply a callback: if you use a callback it will never even allocate any promises so you don't have to be concerned about any performance as some folks who use callbacks tend to be. 🙅

As far as Bluebird.promsify versus the ability to accept a callback or return a promise (the api that this lib is going for) - I'd be pretty surprised if Bluebird.promsify generates any noticeable performance gains over what's going on in this code, but I'll investigate that to see...and like you said if someone prefers that approach nothing is stopping them from doing that as well in their own code on top of the pool. 👍

The goal is to support both promises and callbacks in a transparent way versus requiring someone run Bluebird.promsify or something similar on the pool, at least for the time being: supply a callback & callbacks will be used. Don't & you'll get a promise back. 😄

Thanks again for the feedback & comments. They're very helpful! 👏

[calebboyd]

Thank you for entertaining them. I'm very grateful for your work on these tools 🔨