FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix cluster test for disconnect with open connections by sam-github · Pull Request #1953 · nodejs/node · GitHub

/ node Public

fix cluster test for disconnect with open connections - #1953

Closed
sam-github wants to merge 1 commit into
nodejs:masterfrom
sam-github:pr-1934-redux
ConversationCommits (1)ChecksFiles changed
Closed

fix cluster test for disconnect with open connections#1953
sam-github wants to merge 1 commit into
nodejs:masterfrom
sam-github:pr-1934-redux

Conversation

sam-github commented Jun 11, 2015

Copy link
Copy Markdown
Contributor

fix #1933
replaces #1934

passed all tests except arm7 and raspberry pi 1 and 2, they are still ongoing

@Olegas @trevnorris PTAL

note that I think there are ALSO bugs in cluster message passing... this test attempts to evade them by waiting for the socket to be definitively open... by waiting for data from the worker/server

sam-github force-pushed the pr-1934-redux branch 2 times, most recently from 43a34a9 to 01225df Compare June 11, 2015 21:44

sam-github commented Jun 11, 2015

Copy link
Copy Markdown
Contributor Author

last test: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/814/

re-test after rebase: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/815/

mscdex added cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests. labels Jun 11, 2015

jbergstroem commented Jun 11, 2015

Copy link
Copy Markdown
Member

LGTM. Verified on a machine I could reproduce on prior to patch.

shigeki commented Jun 12, 2015

Copy link
Copy Markdown
Contributor

Timeout error of test-cluster-worker-wait-server-close.js on my FreeBSD machine was resolved with this patch.

shigeki mentioned this pull request Jun 12, 2015
Closed

rvagg commented Jun 12, 2015

Copy link
Copy Markdown
Member

LGTM

test: fix cluster-worker-wait-server-close races
b5621cb 
Wait for data to arrive from worker before doing a disconnect. Without
this, whether the disconnect arrives at the worker before the master
accepts and forwards the connection descriptor to the worker is a race.

sam-github commented Jun 12, 2015

Copy link
Copy Markdown
Contributor Author

Landed in a5987d4d5e21be04a6239579634fa8b19446d5ef

sam-github closed this Jun 12, 2015
sam-github mentioned this pull request Jun 12, 2015
Closed
sam-github deleted the pr-1934-redux branch June 12, 2015 04:58
sam-github mentioned this pull request Jun 12, 2015
Closed

jbergstroem commented Jun 12, 2015

Copy link
Copy Markdown
Member

@sam-github I can't find this commit in the master branch. Github doesn't either. Confuserating.

jbergstroem commented Jun 12, 2015

Copy link
Copy Markdown
Member 
$ git fetch -q iojs && git branch -a --contains a5987d4d5e21be04a6239579634fa8b19446d5ef
error: no such commit a5987d4d5e21be04a6239579634fa8b19446d5ef

jbergstroem commented Jun 12, 2015

Copy link
Copy Markdown
Member

I think I figured out what's going on. You pushed it to your own master branch, but for some reason the generated github link thinks the commit also lives in the nodejs/io.js repo (which it doesn't).

Olegas commented Jun 12, 2015

Copy link
Copy Markdown
Contributor

@sam-github
Test is fine. Passing on my environment. Thanks!
But can't see your commit on master also (
Reopened #1933 just to not get things lost.

shigeki mentioned this pull request Jun 12, 2015
Closed
sam-github added a commit that referenced this pull request Jun 12, 2015
test: fix cluster-worker-wait-server-close races
03ce84d 
Wait for data to arrive from worker before doing a disconnect. Without
this, whether the disconnect arrives at the worker before the master
accepts and forwards the connection descriptor to the worker is a race.

Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Rod Vagg <rod@vagg.org>
PR-URL: #1953
Fixes: #1933
Fixes: #1400

sam-github commented Jun 12, 2015

Copy link
Copy Markdown
Contributor Author

Landed in 03ce84d

Sorry for the confusion

sam-github mentioned this pull request Jun 13, 2015
Closed
rvagg mentioned this pull request Jun 13, 2015
Closed

jbergstroem commented Jun 14, 2015

Copy link
Copy Markdown
Member

Probably not related to this specific test, but part of the same problem: Two freebsd bots saw this issue in a recent run: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/842/

I couldn't find any trailing processes on either machine. Could it possibly be some other test opening it and now being closed by this fix?

sam-github mentioned this pull request Jun 15, 2015
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test: test-cluster-worker-wait-server-close.js is hanging

6 participants

Back | FazBrowse Home | New Git URL