LearnBoost / cluster

Node.JS multi-core server manager with plugins support.
http://learnboost.github.com/cluster
MIT License
2.29k stars 159 forks source link

Previous processes are not killed after restart #38

Closed fabien closed 13 years ago

fabien commented 13 years ago

When starting a cluster in the background (nohup & ...) and then using the cli plugin to restart the cluster, old master and workers processes aren not killed after the new main process has taken over. This can be reproduced with examples/cli.js as follows:

$ nohup node cli.js &
$ node test.js restart
debug - exit
$ node test.js restart
debug - exit
[1]+ Done nohup node test.js
$ node test.js restart
User defined signal 2
$ node test.js restart
User defined signal 2

In the meantime the amount of processes has already doubled/tripled... all the time no (persistent/browser) connections have been made - the above was just executed quickly one command after another.

fabien commented 13 years ago

I tested without sending to background, then from another shell I ran restart. Turns out the restart attempt crashes:

Error: ESRCH, No such process
    at EventEmitter.kill (node.js:203:19)
    at Master.<anonymous> (/node_modules/.npm/cluster/0.2.4/package/lib/master.js:547:13)
tj commented 13 years ago

fails for me as well (which is good haha) looking into it

tj commented 13 years ago

I think I know what the problem is, havent looked yet but I think subsequent restarts do not receive the new master PID so they try to kill the old one set in the environment variable

fabien commented 13 years ago

I was under the same impression, but I couldn't figure out where to get the right PID from to fix it.

tj commented 13 years ago

Fixed master PPID reference. Closed by 5bc31b4d27ade952ddaee93dccbb2043bb93e745

subsequent restarts would not re-assign the PPID due to this precedence bug

tj commented 13 years ago

I found a different bug in the process haha, I was not defaulting SIGQUIT properly after a recent refactor so it was sending KILL for the "graceful" restart haha, not good