-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Exponentially backed-off respawns with maximum attempts #60
base: master
Are you sure you want to change the base?
Conversation
I like the proposed functionality, but in my testing the worker changes to terminated before spawned. As a result, the worker is not added to the workers array, the ~self.workers.indexOf(w) test fails, and the callback (fn) is not called. Moving fn && fn(w.readyState) outside of the if seems to fix this, though I'm not certain it doesn't break anything else. Also, as a result of the initial respawnWorker call being inside that same if, no respawns take place when the first set of workers fail. This is true of the existing keepalive single respawn of course, but it would be nice to deal with this here also. Probably the reason why my workers are 'terminated' before they are 'spawned' is down to using the assumeReady:false and up.ready() for asynchronously declaring readiness of the worker - the worker terminates before that callback fires. |
I have some time tonight and need this functionality for something I'm working on so I'll see if I can take a look and get some tests going. Thanks for having a look. |
Oops... committed on accident and don't feel like fixing. Plz stand by. |
Added tests that check that new events are fired correctly. @fintanf The reason workers are terminated before they're spawned seems to be that when child process workers exit immediately, they're not alive long enough to receive the message that they've been spawned and emit that event. Thus, they go straight from spawning -> terminated which was causing the problem. The PR now includes the original backoff respawns w/max. retries, but also includes the unsuccessful event that fires when a worker doesn't reach it's minimum lifetime. Order now and this pull request can be yours for 3 easy payments of $19.95. Also, the special case of a min. lifetime of 0 is now officially supported and will not emit unsuccessful (with tests). |
This seems to work nicely in the synchronous worker loading case (assumeReady:true). For async worker loading (assumeReady:false) it only retries once though. To recreate you should just have to use assumeReady:false and make your worker throw an error immediately. |
I find this useful when used in concert with minExpectedLifetime: '0s'
Adds 3 new options:
-backoffRespawns: null to disable; -1 to enable with no max. attempts; positive sets maximum attempts
-backoffDelay: node-backoff "initialDelay" option in milli
-backoffMaxDelay: node-backoff ""maxDelay" option in milli
Relies on node-backoff for the backoff implementation.
Sorry... no tests =/