Feature request: Auto-reconnect

[danielgindi]

If I want to instantiate a "knex" object, which is using pool2, and want to make sure that if it fails to connect it will reestablish the connections in the pool - I have to write a lot of extra code.

Currently the code destroys the pool if it never managed to allocate a resource, but maybe we want it to not destroy, but try again...

[myndzi]

I think this is a request for knex itself. The logic behind the decision here is that we DO want resources to automatically get filled/reconnect, but we don't want to silently fail if the configuration is wrong or doesn't work. Thus, an error is thrown when nothing worked and no connection had ever been established -- but after that, resources are indeed essentially "auto reconnected" (that's basically the pool's job).

I can see how this might create a problem if your resource happened to be unavailable when you initially start your application, but in that case all you would have to do is restart the application after fixing the problem, or crash it out and let a managing process manager restart it until it works...?

An option to disable this behavior makes sense, but I would kind of discourage people from using it for the above mentioned reasons.

What is the case you are encountering where you want to retry an initial failure with code?

[danielgindi]

It's a case where multiple systems would not necessarily start up in the order you want them too... MySql can start up in parallel to the node app, and the node app will be available before the MySql server has started. There are other cases too, but this is the classic

[myndzi]

I see what you mean, yeah. That could be a problem if you don't have a robust or useful process management system in place. I myself have hacked a check-mysql delay into some app startups before because it was the most convenient route. Perhaps this would take the form of a time value? You could set it so that the whole thing bombs if no successful connection has been established within a set amount of time. That would allow you to set it to Infinity if you so desired, but would encourage people to choose sane values that meet their requirements, and could default to 0 to maintain compatibility with the current behavior.

[danielgindi]

Yes that's the best, I think. Current behavior should probably stay default for most people, as they need to know the the initial connection has failed. If there's a special case where they need to retry - that's where they alter the configuration.

Maybe there should also be a value of the delay between retries?

[myndzi]

Uncertain. There's already a lot of values to be set, I'd like to avoid loading it down with tons more.

Is there some reason you can't just crash and restart until it works, by the way? I still think that's probably the best solution in your case (and most cases), though I can understand a desire to not want to do that.

[danielgindi]

I think I just don't want the IT guys to worry about it, everything should "just work". Cleaning up and restarting from within the app is too much work for this, and logging crashes could increase error counts when it is ran as a service

• which could cause it to decide to not run again for X minutes based on policies...

[myndzi]

Man, I tried to keep this light and dependency-free but every time I turn around I wish I was using promises. Rate-limiting connection attempts seems like a good idea, but managing it with callbacks is kind of annoying.

[myndzi]

which could cause it to decide to not run again for X minutes based on policies...

That's exactly the "wait until retry" behavior we are discussing, yes? :)

[danielgindi]

Promises will soon be dependency-free as io.js is merging back with node.js, and this includes support for ECMA 6.0 :-)

[myndzi]

ES6 promises are almost as bad as callbacks anyway. Bluebird for the win.

[myndzi]

Anyway, I'll implement something for this; it might be a few days / over the weekend, since this isn't actually in my active work queue at my actual job right now.

[danielgindi]

Well I haven't really compared them yet, I really hope that the final spec will include proper Promises!

[danielgindi]

It's OK I didn't expect you to jump right in and write the code :-) I know how it is to maintain an open-source project... I'll just wait for Github to tell me that the issue is closed! :+1:

[myndzi]

When I say "almost as bad", I just mean as far as using them to do useful things. Libraries like Bluebird add a ton of convenience features that you really miss if all you're writing is "new Promise(function (resolve, reject) { ... })"

[danielgindi]

Well it seems like currently the V8 implementation of Promise is also slower...

[myndzi]

Sorry it took me a bit longer to get around to it than I initially hoped. Can you check out the master branch from github and tell me if it meets your needs?

[danielgindi]

Yes this is perfect! Thank you :-)

[myndzi]

Okay, great. I took care to maintain the existing behavior (tests passed before I wrote the new tests, after I wrote the new code), so it should only be a minor version bump, but I want to run it past @tgriesser before publishing anyway.

[danielgindi]

@tgriesser any news on this? Can we bump the version number?

[myndzi]

I'll just publish it :) It was more of an awareness/caution thing; it should behave as before for all the same configurations, but if by some chance someone reported an issue I wanted him to be aware of the change.