Skip to content

cluster: rewrite debug ports consistently - #7050

Closed
cjihrig wants to merge 1 commit into
nodejs:masterfrom
cjihrig:port
Closed

cluster: rewrite debug ports consistently#7050
cjihrig wants to merge 1 commit into
nodejs:masterfrom
cjihrig:port

Conversation

@cjihrig

Copy link
Copy Markdown
Contributor
Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

cluster

Description of change

When debug flags are passed to clustered applications, the debug port is rewritten for each worker process to avoid collisions. Prior to this commit, each debug flag would get a unique value. This commit reworks the logic to assign the same port value to all debug flags for a single worker.

I'm not 100% sure if this is actually a bug, or desirable behavior. However, the question came up in #6792 (comment), so I decided to take a shot at solving it.

When debug flags are passed to clustered applications, the debug
port is rewritten for each worker process to avoid collisions.
Prior to this commit, each debug flag would get a unique value.
This commit reworks the logic to assign the same port value to
all debug flags for a single worker.
@nodejs-github-botnodejs-github-bot added the cluster Issues and PRs related to the cluster subsystem. label May 29, 2016
@cjihrigcjihrig mentioned this pull request May 29, 2016
@bnoordhuis

Copy link
Copy Markdown
Member

LGTM

I'm not 100% sure if this is actually a bug, or desirable behavior.

I think it qualifies as a bug. debug_port is a global so in the end only a single port is used (last option wins), but with your change it no longer skip ports.

@cjihrig

Copy link
Copy Markdown
ContributorAuthor

@santigimeno

Copy link
Copy Markdown
Member

CI is green. LGTM

cjihrig added a commit that referenced this pull request May 31, 2016
When debug flags are passed to clustered applications, the debug
port is rewritten for each worker process to avoid collisions.
Prior to this commit, each debug flag would get a unique value.
This commit reworks the logic to assign the same port value to
all debug flags for a single worker.
PR-URL: #7050
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@cjihrig

Copy link
Copy Markdown
ContributorAuthor

Thanks for the reviews. Landed in c4f80c1.

@cjihrigcjihrig closed this May 31, 2016
@cjihrig
cjihrig deleted the port branch May 31, 2016 13:22
Fishrock123 pushed a commit that referenced this pull request Jun 1, 2016
When debug flags are passed to clustered applications, the debug
port is rewritten for each worker process to avoid collisions.
Prior to this commit, each debug flag would get a unique value.
This commit reworks the logic to assign the same port value to
all debug flags for a single worker.
PR-URL: #7050
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
When debug flags are passed to clustered applications, the debug
port is rewritten for each worker process to avoid collisions.
Prior to this commit, each debug flag would get a unique value.
This commit reworks the logic to assign the same port value to
all debug flags for a single worker.
PR-URL: #7050
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@MylesBorins

MylesBorins commented Jul 11, 2016

Copy link
Copy Markdown
Contributor

@cjihrig lts?

@cjihrig

Copy link
Copy Markdown
ContributorAuthor

Yes, please.

MylesBorins pushed a commit that referenced this pull request Jul 11, 2016
When debug flags are passed to clustered applications, the debug
port is rewritten for each worker process to avoid collisions.
Prior to this commit, each debug flag would get a unique value.
This commit reworks the logic to assign the same port value to
all debug flags for a single worker.
PR-URL: #7050
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
When debug flags are passed to clustered applications, the debug
port is rewritten for each worker process to avoid collisions.
Prior to this commit, each debug flag would get a unique value.
This commit reworks the logic to assign the same port value to
all debug flags for a single worker.
PR-URL: #7050
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 12, 2016
When debug flags are passed to clustered applications, the debug
port is rewritten for each worker process to avoid collisions.
Prior to this commit, each debug flag would get a unique value.
This commit reworks the logic to assign the same port value to
all debug flags for a single worker.
PR-URL: #7050
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Jul 12, 2016
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
When debug flags are passed to clustered applications, the debug
port is rewritten for each worker process to avoid collisions.
Prior to this commit, each debug flag would get a unique value.
This commit reworks the logic to assign the same port value to
all debug flags for a single worker.
PR-URL: #7050
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 14, 2016
When debug flags are passed to clustered applications, the debug
port is rewritten for each worker process to avoid collisions.
Prior to this commit, each debug flag would get a unique value.
This commit reworks the logic to assign the same port value to
all debug flags for a single worker.
PR-URL: #7050
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

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