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.

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

test: try other ipv6 localhost alternatives #4325

Closed

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Dec 17, 2015

ubuntu1404-64 has been having issues with test-net-connect-options-ipv6 fairly frequently. I'm not exactly sure why the IPv6 localhost resolving is flaky on that particular machine (it could be the way its network is configured?), but being an Ubuntu system it typically has other IPv6 localhost hostnames.

This change tries those special IPv6 hostnames first before trying localhost. If those special hostnames fail to resolve and if localhost should error with ENOTFOUND, it retries several times before giving up.

I'm hoping this is enough to fix things. I did perform a stress test on that particular node and it was ok on every run after this change.

@mscdex mscdex added net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests. labels Dec 17, 2015
@mscdex mscdex force-pushed the fix-flaky-net-connect-options-ipv6 branch from 5c0cfa1 to 878f245 Compare December 17, 2015 09:56
@bnoordhuis
Copy link
Member

I don't know about this PR. The test queries for 'localhost' with family=AF_INET6. If the local resolver doesn't know how to resolve that, I would argue it's the resolver that is broken, not the test.

The change itself LGTM except that there should be two spaces before comments.

@jasnell
Copy link
Member

jasnell commented Dec 17, 2015

Generally agree but it seems worthwhile to be a bit flexible and work around broken resolvers.
LGTM

@mscdex mscdex force-pushed the fix-flaky-net-connect-options-ipv6 branch from 878f245 to a924136 Compare December 17, 2015 17:00
@mscdex
Copy link
Contributor Author

mscdex commented Dec 17, 2015

I adjusted the comments a bit, LGTY @bnoordhuis ?

@bnoordhuis
Copy link
Member

LGTM if the CI likes it.

@mscdex
Copy link
Contributor Author

mscdex commented Dec 17, 2015

mscdex added a commit that referenced this pull request Dec 17, 2015
PR-URL: #4325
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mscdex
Copy link
Contributor Author

mscdex commented Dec 17, 2015

Landed in 852313a.

@mscdex mscdex closed this Dec 17, 2015
@mscdex mscdex deleted the fix-flaky-net-connect-options-ipv6 branch December 17, 2015 17:55
@jasnell jasnell mentioned this pull request Dec 17, 2015
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Dec 22, 2015
PR-URL: nodejs#4325
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
PR-URL: nodejs#4325
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
mscdex added a commit that referenced this pull request Jan 7, 2016
PR-URL: #4325
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
PR-URL: #4325
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
PR-URL: nodejs#4325
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants