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.

Sign up for GitHub

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

Jump to bottom

fix cluster test for disconnect with open connections #1953

Closed
wants to merge 1 commit into from
Closed

fix cluster test for disconnect with open connections #1953

wants to merge 1 commit into from

Conversation

sam-github
Copy link
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 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
Copy link
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 mscdex added cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests. labels Jun 11, 2015
@jbergstroem
Copy link
Member

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

@shigeki
Copy link
Contributor

shigeki commented Jun 12, 2015

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

@shigeki shigeki mentioned this pull request Jun 12, 2015
Closed
@rvagg
Copy link
Member

rvagg commented Jun 12, 2015

LGTM

@sam-github
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
Copy link
Contributor Author

Landed in a5987d4d5e21be04a6239579634fa8b19446d5ef

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

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

@jbergstroem
Copy link
Member 

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

@jbergstroem
Copy link
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
Copy link
Contributor

Olegas commented Jun 12, 2015

@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 shigeki mentioned this pull request Jun 12, 2015
Closed
sam-github added a commit that referenced this pull request Jun 12, 2015
@sam-github
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 <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
PR-URL: #1953
Fixes: #1933
Fixes: #1400
@sam-github
Copy link
Contributor Author

Landed in 03ce84d

Sorry for the confusion

@sam-github sam-github mentioned this pull request Jun 13, 2015
Closed
@rvagg rvagg mentioned this pull request Jun 13, 2015
Closed
@jbergstroem
Copy link
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 sam-github mentioned this pull request Jun 15, 2015
Closed
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
@sam-github @jbergstroem @shigeki @rvagg @Olegas @mscdex