Skip to content

cluster: support stdio option for workers - #7838

Merged
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:stdio
Aug 1, 2016
Merged

cluster: support stdio option for workers#7838
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:stdio

Conversation

@cjihrig

Copy link
Copy Markdown
Contributor
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-botnodejs-github-bot added the cluster Issues and PRs related to the cluster subsystem. label Jul 22, 2016
@cjihrig

Copy link
Copy Markdown
ContributorAuthor

@santigimeno

Copy link
Copy Markdown
Member

LGTM

Comment threaddoc/api/cluster.md Outdated

@bnoordhuisbnoordhuisJul 23, 2016

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
ContributorAuthor

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.

@jasnelljasnell 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
Copy Markdown
Member

LGTM with a comment

@cjihrig

Copy link
Copy Markdown
ContributorAuthor

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:masterAug 1, 2016
@cjihrig
cjihrig deleted the stdio branch August 1, 2016 19:10
@cjihrigcjihrig 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>
@cjihrigcjihrig 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clusterIssues and PRs related to the cluster subsystem.semver-minorPRs 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

@cjihrig@santigimeno@jasnell@bnoordhuis@MylesBorins@nodejs-github-bot