Skip to content

Conversation

@cjihrig
Copy link
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
ContributorAuthor

@santigimeno
Copy link
Member

LGTM

Copy link
Member

@bnoordhuisbnoordhuisJul 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
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
Member

LGTM with a comment

@cjihrig
Copy link
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 <[email protected]> Reviewed-By: James M Snell <[email protected]>
@cjihrigcjihrig merged commit 75c6d9d into nodejs:masterAug 1, 2016
@cjihrigcjihrig 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 <[email protected]> Reviewed-By: James M Snell <[email protected]>
@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