Skip to content
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

Debug call illustration in the cluster doc does not match with output in Windows #10255

Closed
vsemozhetbyt opened this issue Dec 13, 2016 · 2 comments
Labels
cluster Issues and PRs related to the cluster subsystem. doc Issues and PRs related to the documentations.

Comments

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Dec 13, 2016

  • Version: Node.js 7.2.1
  • Platform: Windows 7 x64
  • Subsystem: cluster, doc

In the preface of the cluster doc the first code example is followed by a shell illustration of a debug script call:

$ NODE_DEBUG=cluster node server.js
23521,Master Worker 23524 online
23521,Master Worker 23526 online
23521,Master Worker 23523 online
23521,Master Worker 23528 online

However, I can't get the same output in Windows:

J:\temp>type server.js
const cluster = require('cluster');
const http = require('http');
const numCPUs = require('os').cpus().length;

if (cluster.isMaster) {
  // Fork workers.
  for (var i = 0; i < numCPUs; i++) {
    cluster.fork();
  }

  cluster.on('exit', (worker, code, signal) => {
    console.log(`worker ${worker.process.pid} died`);
  });
} else {
  // Workers can share any TCP connection
  // In this case it is an HTTP server
  http.createServer((req, res) => {
    res.writeHead(200);
    res.end('hello world\n');
  }).listen(8000);
}
J:\temp>SET NODE_DEBUG=cluster && node server.js

// no debug output;
// process monitor shows all four clusters are running;
// servers respond successfully.

// press Ctrl+C

^C

J:\temp>

// environment variable check

J:\temp>SET NODE_DEBUG

NODE_DEBUG=cluster

J:\temp>

I've tried to find in the cluster.js any checks for NODE_DEBUG or any util.debuglog() calls and I could not find any.

Is this illustration still valid?
Is it not valid only for Windows?
Is this note after the illustration supposed to explain the output difference in Windows:

Please note that on Windows, it is not yet possible to set up a named pipe server in a worker.

Should the document fragment be somehow clarified more?

@sam-github
Copy link
Contributor

The use of NODE_DEBUG should be removed and the workers should print the log message directly from the example code right before or after createServer()

The debug statements are gone now since the 0.12 cluster rewrite.

Feel free to PR an improvement.

@sam-github sam-github added cluster Issues and PRs related to the cluster subsystem. doc Issues and PRs related to the documentations. labels Dec 13, 2016
@vsemozhetbyt
Copy link
Contributor Author

@sam-github Done. Thank you.

evanlucas pushed a commit that referenced this issue Jan 3, 2017
- Fixes #10255
- It also will be consistent with a previous code example.
- `cluster.workers` iteration: `Object.keys().forEach` -> `for`...`in`

PR-URL: #10270
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
evanlucas pushed a commit that referenced this issue Jan 4, 2017
- Fixes #10255
- It also will be consistent with a previous code example.
- `cluster.workers` iteration: `Object.keys().forEach` -> `for`...`in`

PR-URL: #10270
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 23, 2017
- Fixes #10255
- It also will be consistent with a previous code example.
- `cluster.workers` iteration: `Object.keys().forEach` -> `for`...`in`

PR-URL: #10270
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 24, 2017
- Fixes #10255
- It also will be consistent with a previous code example.
- `cluster.workers` iteration: `Object.keys().forEach` -> `for`...`in`

PR-URL: #10270
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
MylesBorins pushed a commit that referenced this issue Jan 31, 2017
- Fixes #10255
- It also will be consistent with a previous code example.
- `cluster.workers` iteration: `Object.keys().forEach` -> `for`...`in`

PR-URL: #10270
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cluster Issues and PRs related to the cluster subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

No branches or pull requests

2 participants