brianc / node-forky Goto Github PK

Hey, I'm not sure, but I think there's a little bug in the disconnect function.

When a client repeatedly hits a route that throws an error (that results in disconnecting the worker), I get the following error:

Error: channel closed
18:09:12 web.1  |     at process.target.send (child_process.js:406:26)
18:09:12 web.1  |     at Worker.send (cluster.js:406:21)
18:09:12 web.1  |     at Function.forky.disconnect (./node_modules/forky/index.js:77:12)

When the client hits the route the first time, the worker will get disconnected but the connection to the client remains. If the client hits the route again the worker will try to send another disconnect message to the master. Since the channel is closed, the above error will occur.

In the disconnect function the following lines (I guess) should prevent this behavior:

if(worker.state == 'disconnecting') return;
  worker.state = 'disconnecting';

The worker however will set its state to disconnected (not disconnecting), which will lead the worker to send another message.

Correct me If I'm wrong!?

Maybe the above lines should be changed to:

if(worker.state == 'disconnecting' || worker.state == 'disconnected') return;
  worker.state = 'disconnecting';

What do you think?

Best regards!

worker.kill doesn't exist in 0.8.x of node. Please use worker.destroy instead which is supported in 0.8.x and 0.10.x

Hi!

Thanks for this great library.
Unfortunately, while trying to use it on Heroku, the dyno is getting killed because it's exceeding the memory quota. This is the exact log:

2014-09-03T19:55:38.416261+00:00 heroku[web.1]: Process running mem=523M(102.2%)
2014-09-03T19:55:38.416525+00:00 heroku[web.1]: Error R14 (Memory quota exceeded)

It works fine for a while (no error logs), and then it starts exceeding the quota in a loop, until I restart Heroku and everything is back to normal for a while.

app[web.1]: info: <anonymous@{removed}>: HTTP HEAD / name=QI, hostname={removed}, pid=7, time=Wed Sep 03 2014 19:49:31 GMT+0000 (UTC)
heroku[router]: at=info method=HEAD path="/" host=www.qilabs.org request_id={removed} fwd={removed} dyno=web.1 connect=2ms service=15ms status=200 bytes=429
heroku[router]: at=info method=HEAD path="/" host=www.qilabs.org request_id={removed} fwd={removed} dyno=web.1 connect=4ms service=14ms status=200 bytes=428
app[web.1]: info: <anonymous@{removed}>: HTTP HEAD / name=QI, hostname={removed}, pid=6, time=Wed Sep 03 2014 19:49:31 GMT+0000 (UTC)
app[web.1]: info: <anonymous@{removed}>: HTTP HEAD / name=QI, hostname={removed}, pid=5, time=Wed Sep 03 2014 19:49:35 GMT+0000 (UTC)
heroku[router]: at=info method=HEAD path="/" host=www.qilabs.org request_id=39f79216-d71f-40c0-8a59-464325425c1e fwd={removed} dyno=web.1 connect=2ms service=261ms status=200 bytes=429
heroku[web.1]: Process running mem=512M(100.1%)
heroku[web.1]: Error R14 (Memory quota exceeded)
heroku[web.1]: Process running mem=512M(100.1%)
heroku[web.1]: Error R14 (Memory quota exceeded)
heroku[router]: at=info method=HEAD path="/" host=www.qilabs.org request_id={removed} fwd={removed} dyno=web.1 connect=1ms service=8ms status=200 bytes=430
app[web.1]: info: <anonymous@{removed}>: HTTP HEAD / name=QI, hostname={removed}, pid=4, time=Wed Sep 03 2014 19:50:12 GMT+0000 (UTC)
heroku[router]: at=info method=HEAD path="/" host=www.qilabs.org request_id={removed} fwd={removed} dyno=web.1 connect=0ms service=10ms status=200 bytes=430
app[web.1]: info: <anonymous@{removed}>: HTTP HEAD / name=QI, hostname={removed}, pid=4, time=Wed Sep 03 2014 19:50:24 GMT+0000 (UTC)
heroku[web.1]: Process running mem=512M(100.1%)
heroku[web.1]: Error R14 (Memory quota exceeded)
heroku[router]: at=info method=HEAD path="/" host=www.qilabs.org request_id=261f26a7-e5e4-4fba-81a6-ebf427e1257b fwd={removed} dyno=web.1 connect=1ms service=14ms status=200 bytes=429
app[web.1]: info: <anonymous@{removed}>: HTTP HEAD / name=QI, hostname={removed}, pid=4, time=Wed Sep 03 2014 19:50:34 GMT+0000 (UTC)
heroku[web.1]: Process running mem=513M(100.3%)
heroku[web.1]: Error R14 (Memory quota exceeded)
heroku[web.1]: Process running mem=513M(100.3%)
heroku[web.1]: Error R14 (Memory quota exceeded)
heroku[web.1]: Process running mem=514M(100.5%)
heroku[web.1]: Error R14 (Memory quota exceeded)
heroku[web.1]: Process running mem=514M(100.5%)
heroku[web.1]: Error R14 (Memory quota exceeded)
heroku[web.1]: Process running mem=514M(100.6%)
heroku[web.1]: Error R14 (Memory quota exceeded)
(and repeat)

(the HEAD requests are newrelic's)

I seem to be following all the steps as indicated in the README. Perhaps the memory leaks are my fault, but I've never seen these messages before using forky. I'll try not using the cluster for a while, and see if the errors still occur.

Thoughts, anyone?

Forky makes it difficult to operate graceful shutdowns in environments such as Heroku that blast all processes with SIGTERM on shutdown.

Forky has a flag shuttingDown that if it exported directly or via a setter function to allow it to be set to true, it could prevent the restart of child processes during the exit.

Is there a reason the flag shuttingDown is not exposed and seems to be a defacto constant? It might be a bug.

It's a real easy fix to allow a setter for shuttingDown and I'd put up a PR for that if it's something the admins would agree to.

Hi @brianc,

Forky is great — we've been using it for two years now. Thank you!

One question: a couple of times now, we've accidentally shipped a bug where we hit a runtime error on startup. Forky sees the process crash and restarts it. But this just repeats ad infinitum.

Because the master Forky process itself doesn't fail, we don't detect any error immediately. The good news is that we do eventually detect a failure after a brief time threshold. (Heroku sees that we haven't bound to the network port after a minute ==> R10 error.)

Just curious: is there any way Forky could detect an error on startup, whether exactly or heuristically? E.g. an error <N seconds from spawn, or >N errors in <M seconds, or similar?

Thanks!

master.js

require('forky')({ path: __dirname + '/worker.js' }, 1 );

worker.js

console.log( 'Hello from worker', require('cluster').worker.id );
require('http').createServer( function( req, res ){ } );

node master

Hello from worker 1
Hello from worker 3
Hello from worker 2
Hello from worker 4

If you change the master.js to:

require('forky')( __dirname + '/worker.js', 1 );

it works fine

2legit2quit

// please add optional count variable to be able to set a custom number of workers ;-)

forky = module.exports = function (path, cb, count) {
cluster.setupMaster({
exec: path
});
var cores = count || os.cpus().length;