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

cluster: support stdio option for workers #7838

Merged
merged 1 commit into from
Aug 1, 2016
Merged

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Jul 22, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

cluster

Description of change

This commit allows setupMaster() to configure the stdio channels for worker processes.

Refs: nodejs/node-v0.x-archive#5727
Refs: #7811

@nodejs-github-bot nodejs-github-bot added the cluster Issues and PRs related to the cluster subsystem. label Jul 22, 2016
@cjihrig
Copy link
Contributor Author

cjihrig commented Jul 22, 2016

@santigimeno
Copy link
Member

LGTM

@@ -626,6 +626,8 @@ values are `"rr"` and `"none"`.
(Default=`process.argv.slice(2)`)
* `silent` {Boolean} whether or not to send output to parent's stdio.
(Default=`false`)
* `stdio` {Array} Configures the stdio of forked processes. When this option
is provided, it overrides `silent`.
Copy link
Member

@bnoordhuis bnoordhuis Jul 23, 2016

Choose a reason for hiding this comment

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

This should explain that fd 3 must be 'ipc', otherwise the cluster module won't work. It's unfortunate in that it leaks what is essentially an implementation detail.

EDIT: Or at least one 'ipc' element, if I read #7811 right.

Copy link
Member

Choose a reason for hiding this comment

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

hmm... perhaps the implementation should simply override or error if ipc is not passed. If overridden, a warning can be emitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will throw in child_process.fork() if there is no IPC. I'll update these docs accordingly.

@jasnell jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 26, 2016
@jasnell
Copy link
Member

jasnell commented Jul 26, 2016

LGTM with a comment

@cjihrig
Copy link
Contributor Author

cjihrig commented Aug 1, 2016

It's been a while, so another CI before landing: https://ci.nodejs.org/job/node-test-pull-request/3488/

This commit allows setupMaster() to configure the stdio channels
for worker processes.

Refs: nodejs/node-v0.x-archive#5727
Refs: nodejs#7811
PR-URL: nodejs#7838
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig merged commit 75c6d9d into nodejs:master Aug 1, 2016
@cjihrig cjihrig deleted the stdio branch August 1, 2016 19:10
@cjihrig cjihrig mentioned this pull request Aug 8, 2016
cjihrig added a commit that referenced this pull request Aug 10, 2016
This commit allows setupMaster() to configure the stdio channels
for worker processes.

Refs: nodejs/node-v0.x-archive#5727
Refs: #7811
PR-URL: #7838
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig mentioned this pull request Aug 11, 2016
cjihrig added a commit that referenced this pull request Aug 11, 2016
Notable changes:

* build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576
* child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838
* fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942
* repl: The REPL now supports editor mode. (Prince J Wesley) #7275
* util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013

Refs: #8020
PR-URL: #8070
cjihrig added a commit that referenced this pull request Aug 11, 2016
Notable changes:

* build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576
* child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838
* child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) #7696
* fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942
* repl: The REPL now supports editor mode. (Prince J Wesley) #7275
* util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013

Refs: #8020
PR-URL: #8070
cjihrig added a commit that referenced this pull request Aug 15, 2016
Notable changes:

* build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) #7983 and #7576
* child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) #7811 and #7838
* child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) #7696
* fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) #7942
* repl: The REPL now supports editor mode. (Prince J Wesley) #7275
* util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) #8013

Refs: #8020
PR-URL: #8070
cjihrig added a commit to cjihrig/node that referenced this pull request Aug 16, 2016
Notable changes:

* build: zlib symbols and additional OpenSSL symbols are now exposed on Windows platforms. (Alex Hultman) nodejs#7983 and nodejs#7576
* child_process, cluster: Forked child processes and cluster workers now support stdio configuration. (Colin Ihrig) nodejs#7811 and nodejs#7838
* child_process: argv[0] can now be set to arbitrary values in spawned processes. (Pat Pannuto) nodejs#7696
* fs: fs.ReadStream now exposes the number of bytes it has read so far. (Linus Unnebäck) nodejs#7942
* repl: The REPL now supports editor mode. (Prince J Wesley) nodejs#7275
* util: inspect() can now be configured globally using util.inspect.defaultOptions. (Roman Reiss) nodejs#8013

Refs: nodejs#8020
PR-URL: nodejs#8070
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. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants