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

doc: improvements to cluster.markdown copy #4409

Closed
wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Dec 24, 2015

A number of general improvements to cluster.markdown including
copyedits and revised examples

@jasnell jasnell added cluster Issues and PRs related to the cluster subsystem. doc Issues and PRs related to the documentations. lts-watch-v4.x labels Dec 24, 2015
### Event: 'exit'

* `worker` {Worker object}
* `code` {Number} the exit code, if it exited normally.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"terminated normally" or just "exited" would be more consistent with child process docs

@jasnell
Copy link
Member Author

jasnell commented Dec 25, 2015

@sam-github ... updated to address #4409 (comment)

A single instance of Node.js runs in a single thread. To take advantage of
multi-core systems the user will sometimes want to launch a cluster of Node.js
processes to handle the load.
All JavaScript running within a single instance of Node.js executes within the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first sentence is a bit long in the tooth and hard to understand. Maybe the same thing can be said with less.

"Node.js executes JavaScript code within the context of a single-threaded event loop."

@jasnell
Copy link
Member Author

jasnell commented Dec 29, 2015

@ryansobol ... fixed!

A single instance of Node.js runs in a single thread. To take advantage of
multi-core systems the user will sometimes want to launch a cluster of Node.js
processes to handle the load.
Node.js executes JavaScript code within the context of a single-threaded event
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a stab at making the cluster introduction more approachable to beginners. I was able to consolidate three paragraphs down to two which is a win with respects to the reader quickly seeing their first code example. In addition, I tried to answer why clusters are important by first explaining the benefits and shortcomings of libuv concurrency model at a high level.

In the process, the references about child_process.fork() and IPC channels was cut. In my experience as a teacher, a newcomer wants to know three things in the beginning—what a thing is, why it's important, and how to use it. Implementation details can come later.

Anyway, feel free to use none, some, or all of it. :)

Edit: Remembered that the cluster API's primarily used for non-blocking socket I/O bound apps. While I'm sure it's also useful for CPU bound apps too, it's probably better to focus on the primary use case.


Node.js executes JavaScript code within the context of an event loop. Under the hood, non-blocking socket I/O runs on the main event loop thread while blocking DNS and filesystem I/O runs on a secondary thread that notifies the main thread when it's finished. This concurrency model allows a blocking I/O bound application to easily distribute work across multiple CPU cores. For more details, see the libuv project.

The cluster API allows a non-blocking I/O bound application to distribute its work across multiple CPU cores. A Node.js cluster is created when a master process forks one or more worker processes. By design, workers use the same application code as the master. Conveniently, the cluster.isMaster property determines if a running process is the master or a worker.

const cluster = require('cluster');
if (cluster.isMaster) {
  for (var i = 0; i < numCPUs; i++) {
    cluster.fork();
  }
} else {
  // Non-blocking I/O work
}

@jasnell
Copy link
Member Author

jasnell commented Dec 30, 2015

Updated to use @ryansobol's wording for the intro

@jasnell
Copy link
Member Author

jasnell commented Jan 5, 2016

@nodejs/documentation @sam-github @ryansobol ... would appreciate one more quick review on this before landing.

all share server ports.
The `cluster` API allows a non-blocking I/O bound application to distribute its work across multiple CPU cores. A Node.js cluster is created when a **master process** forks one or more **worker processes**. By design, workers use the same application code as the master. Conveniently, the `cluster.isMaster` property determines if a running process is the master or a worker.

const cluster = require('cluster');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add const numCPUs = require('os').cpus().length;?

@jasnell
Copy link
Member Author

jasnell commented Jan 15, 2016

@ryansobol ... thanks for the comments! I'll update shortly

shigeki pushed a commit to shigeki/node that referenced this pull request Jan 30, 2016
Per nodejs#4409, the documentation on http.abort is a bit lacking.
This provides a slight improvement.

Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node-v0.x-archive#25591
@benjamingr
Copy link
Member

Hey @jasnell are you still working on these? Need a review?

@jasnell
Copy link
Member Author

jasnell commented Mar 22, 2016

@benjamingr ... yeah, I still have a few of these queued up. I plan to revisit them hopefully later this week.

A number of general improvements to cluster.markdown including
copyedits and revised examples
@jasnell
Copy link
Member Author

jasnell commented Mar 22, 2016

@benjamingr @ryansobol ... ping.. rebased and updated!
@nodejs/documentation

```js
const cluster = require('cluster');
if (cluster.isMaster) {
for (var i = 0; i < numCPUs; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally prefer let in loops but I see both var or let in the docs. (https://github.com/nodejs/node/blob/master/doc/api/buffer.markdown has both)
Excuse me if this has been brought up before.

@stevemao
Copy link
Contributor

stevemao commented May 4, 2016

So this is not going to land?

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

Successfully merging this pull request may close these issues.

6 participants