Cluster 2.0 – step 5 : Make setupMaster public API #2470

Closed
wants to merge 8 commits into
from

Conversation

Projects
None yet
2 participants
Member

AndreasMadsen commented Jan 5, 2012

This make the setupMaster public API.

It allow:

  • workers to not relay stderr and stdout data
  • Custom child args
  • To have workers and master in separate files.

When the cluster is initialized a setup event is emitted and the cluster.settings is set. This allow module (cluster plugins) to know the cluster settings there was used.

@mscdex @visionmedia @ry please review

This is about the merging in the cluster 2.0 ( see #2038 ) pull request

@ry ry commented on an outdated diff Jan 5, 2012

test/simple/test-cluster-master-error.js
+
+ // throw accidently error
+ process.nextTick(function() {
+ throw 'accidently error';
+ });
+ }
+
+ });
+
+ // Startup a basic cluster
+ cluster.fork();
+ cluster.fork();
+}
+
+// This is the testcase
+else {
@ry

ry Jan 5, 2012

style should be:

} else {
  // This is the testcase

@ry ry and 1 other commented on an outdated diff Jan 5, 2012

test/simple/test-cluster-master-error.js
+
+ // Spawn a cluster process
+ var master = fork(process.argv[1], ['cluster'], {silent: true});
+
+ // relay output using only stdout
+ master.stdout.on('data', function(data) {
+ data.toString().split('\n').forEach(function(text) {
+ console.log('stdout: ' + text);
+ });
+ });
+
+ master.stderr.on('data', function(data) {
+ data.toString().split('\n').forEach(function(text) {
+ console.log('stderr: ' + text);
+ });
+ });
@ry

ry Jan 5, 2012

if the master is silent - shouldn't it not be outputting stuff to stdout and stderr?

@AndreasMadsen

AndreasMadsen Jan 5, 2012

Member

This testcase is not about {silent: true}. But it did confused make test if I did not set it. Anyway this was only for debugging purpose, I will just remove it.

@ry ry commented on an outdated diff Jan 5, 2012

test/simple/test-cluster-setup-master.js
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+
+var common = require('../common');
+var assert = require('assert');
+var cluster = require('cluster');
+
+if (cluster.isWorker) {
+
+ //Just keep the worker alive
+ process.send(process.argv[2]);
+}
+
+else if (cluster.isMaster) {
Member

AndreasMadsen commented Jan 6, 2012

@ry fixed it all :)

Member

AndreasMadsen commented Jan 17, 2012

I will update this pull request to fix #2556

Member

AndreasMadsen commented Jan 17, 2012

Remember to close #2420

ry closed this in f9a47de Jan 20, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment