Skip to content

Commit

Permalink
More cluster event consistency
Browse files Browse the repository at this point in the history 
Regarding discussion in nodejs#3198.  Passing the worker as an argument
to an event emitted on the worker is redundant, and an unnecessary
break in consistency vs the events on the ChildProcess objects.

It was removed from 'exit', but 'listening' and others were
overlooked.  This corrects that oversight.
  • Loading branch information
@isaacs
isaacs committed May 5, 2012
1 parent 1930772 commit 3d84c3d
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 43 deletions.
60 changes: 29 additions & 31 deletions doc/api/cluster.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ all share server ports.
cluster.fork();
}

cluster.on('exit', function(worker) {
cluster.on('exit', function(worker, code, signal) {
console.log('worker ' + worker.pid + ' died');
});
} else {
Expand Down Expand Up @@ -77,17 +77,17 @@ When a new worker is forked the cluster module will emit a 'fork' event.
This can be used to log worker activity, and create you own timeout.

var timeouts = [];
var errorMsg = function () {
function errorMsg() {
console.error("Something must be wrong with the connection ...");
});
}

cluster.on('fork', function (worker) {
cluster.on('fork', function(worker) {
timeouts[worker.uniqueID] = setTimeout(errorMsg, 2000);
});
cluster.on('listening', function (worker) {
cluster.on('listening', function(worker, address) {
clearTimeout(timeouts[worker.uniqueID]);
});
cluster.on('exit', function (worker) {
cluster.on('exit', function(worker, code, signal) {
clearTimeout(timeouts[worker.uniqueID]);
errorMsg();
});
Expand All @@ -102,7 +102,7 @@ The difference between 'fork' and 'online' is that fork is emitted when the
master tries to fork a worker, and 'online' is emitted when the worker is
being executed.

cluster.on('online', function (worker) {
cluster.on('online', function(worker) {
console.log("Yay, the worker responded after it was forked");
});

Expand All @@ -120,7 +120,7 @@ object and the `address` object contains the following connection properties:
`address`, `port` and `addressType`. This is very useful if the worker is listening
on more than one address.

cluster.on('listening', function (worker, address) {
cluster.on('listening', function(worker, address) {
console.log("A worker is now connected to " + address.address + ":" + address.port);
});

Expand All @@ -143,11 +143,14 @@ connections.
## Event: 'exit'

* `worker` {Worker object}
* `code` {Number} the exit code, if it exited normally.
* `signal` {String} the name of the signal (eg. `'SIGHUP'`) that caused
the process to be killed.

When any of the workers die the cluster module will emit the 'exit' event.
This can be used to restart the worker by calling `fork()` again.

cluster.on('exit', function(worker) {
cluster.on('exit', function(worker, code, signal) {
var exitCode = worker.process.exitCode;
console.log('worker ' + worker.pid + ' died ('+exitCode+'). restarting...');
cluster.fork();
Expand Down Expand Up @@ -225,14 +228,14 @@ In the cluster all living worker objects are stored in this object by there
callback(cluster.workers[uniqueID]);
}
}
eachWorker(function (worker) {
eachWorker(function(worker) {
worker.send('big announcement to all workers');
});

Should you wish to reference a worker over a communication channel, using
the worker's uniqueID is the easiest way to find the worker.

socket.on('data', function (uniqueID) {
socket.on('data', function(uniqueID) {
var worker = cluster.workers[uniqueID];
});

Expand Down Expand Up @@ -285,7 +288,7 @@ This example will echo back all messages from the master:
worker.send('hi there');

} else if (cluster.isWorker) {
process.on('message', function (msg) {
process.on('message', function(msg) {
process.send(msg);
});
}
Expand All @@ -296,7 +299,7 @@ This function will kill the worker, and inform the master to not spawn a
new worker. The boolean `suicide` lets you distinguish between voluntary
and accidental exit.

cluster.on('exit', function (worker) {
cluster.on('exit', function(worker, code, signal) {
if (worker.suicide === true) {
console.log('Oh, it was just suicide\' – no need to worry').
}
Expand Down Expand Up @@ -324,30 +327,30 @@ that would normally not allow the worker to do any cleanup if needed.
var worker = cluser.fork();
var timeout;

worker.on('listening', function () {
worker.on('listening', function(address) {
worker.disconnect();
timeout = setTimeout(function () {
timeout = setTimeout(function() {
worker.send('force kill');
}, 2000);
});

worker.on('disconnect', function () {
worker.on('disconnect', function() {
clearTimeout(timeout);
});

} else if (cluster.isWorker) {
var net = require('net');
var server = net.createServer(function (socket) {
var server = net.createServer(function(socket) {
// connection never end
});

server.listen(8000);

server.on('close', function () {
server.on('close', function() {
// cleanup
});

process.on('message', function (msg) {
process.on('message', function(msg) {
if (msg === 'force kill') {
server.destroy();
}
Expand Down Expand Up @@ -377,15 +380,15 @@ in the master process using the message system:
}, 1000);

// Count requestes
var messageHandler = function (msg) {
function messageHandler(msg) {
if (msg.cmd && msg.cmd == 'notifyRequest') {
numReqs += 1;
}
};
}

// Start workers and listen for messages containing notifyRequest
cluster.autoFork();
Object.keys(cluster.workers).forEach(function (uniqueID) {
Object.keys(cluster.workers).forEach(function(uniqueID) {
cluster.workers[uniqueID].on('message', messageHandler);
});

Expand All @@ -403,35 +406,30 @@ in the master process using the message system:

### Event: 'online'

* `worker` {Worker object}

Same as the `cluster.on('online')` event, but emits only when the state change
on the specified worker.

cluster.fork().on('online', function (worker) {
cluster.fork().on('online', function() {
// Worker is online
};

### Event: 'listening'

* `worker` {Worker object}
* `address` {Object}

Same as the `cluster.on('listening')` event, but emits only when the state change
on the specified worker.

cluster.fork().on('listening', function (worker, address) {
cluster.fork().on('listening', function(address) {
// Worker is listening
};

### Event: 'disconnect'

* `worker` {Worker object}

Same as the `cluster.on('disconnect')` event, but emits only when the state change
on the specified worker.

cluster.fork().on('disconnect', function (worker) {
cluster.fork().on('disconnect', function() {
// Worker has disconnected
};

Expand All @@ -445,7 +443,7 @@ Emitted by the individual worker instance, when the underlying child process
is terminated. See [child_process event: 'exit'](child_process.html#child_process_event_exit).

var worker = cluster.fork();
worker.on('exit', function (code, signal) {
worker.on('exit', function(code, signal) {
if( signal ) {
console.log("worker was killed by signal: "+signal);
} else if( code !== 0 ) {
Expand Down
6 changes: 3 additions & 3 deletions lib/cluster.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ if (cluster.isMaster) {
messageHandler.online = function(message, worker) {
worker.state = 'online';
debug('Worker ' + worker.process.pid + ' online');
worker.emit('online', worker);
worker.emit('online');
cluster.emit('online', worker);
};

Expand Down Expand Up @@ -213,7 +213,7 @@ if (cluster.isMaster) {
worker.state = 'listening';

// Emit listening, now that we know the worker is listening
worker.emit('listening', worker, {
worker.emit('listening', {
address: message.address,
port: message.port,
addressType: message.addressType
Expand Down Expand Up @@ -297,7 +297,7 @@ function Worker(customEnv) {
this.process.once('exit', function(exitCode, signalCode) {
prepareExit(self, 'dead');
self.emit('exit', exitCode, signalCode);
cluster.emit('exit', self);
cluster.emit('exit', self, exitCode, signalCode);
});
this.process.once('disconnect', function() {
prepareExit(self, 'disconnected');
Expand Down
30 changes: 21 additions & 9 deletions test/simple/test-cluster-basic.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,26 @@ else if (cluster.isMaster) {
checks.worker.events[name] = true;

//Check argument
if (name == 'exit') {
checks.worker.equal[name] = (
worker.process.exitCode === arguments[0] &&
worker.process.signalCode === arguments[1] &&
worker === this
);
} else {
checks.worker.equal[name] = worker === arguments[0];
checks.worker.equal[name] = (worker === this);

switch (name) {
case 'exit':
assert.equal(arguments[0], worker.process.exitCode);
assert.equal(arguments[1], worker.process.signalCode);
assert.equal(arguments.length, 2);
break;

case 'listening':
assert.equal(arguments.length, 1);
var expect = { address: '127.0.0.1',
port: common.PORT,
addressType: 4 };
assert.deepEqual(arguments[0], expect);
break;

default:
assert.equal(arguments.length, 0);
break;
}
});
});
Expand All @@ -145,7 +157,7 @@ else if (cluster.isMaster) {
//Check cluster event arguments
forEach(checks.cluster.equal, function(check, name) {
assert.ok(check, 'The cluster event "' + name + '" did not emit ' +
'with corrent argument');
'with correct argument');
});

//Check worker states
Expand Down

0 comments on commit 3d84c3d

Please sign in to comment.