-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Optionally fallback to IPv4 when IPv6 is unreachable #2662
Conversation
4e8ecdb
to
fdec324
Compare
Bundler tests are failing. How do I run bundler tests in context of my rubygems changes? |
I think if you switch to the |
You need to set the |
I think it should just work because bundler's Rakefile already detects the rubygems checkout: https://github.com/bundler/bundler/blob/master/Rakefile#L9-L14. No need to set the |
@deivid-rodriguez it didn't for me, i just tried it since i'm testing a patch in bundler against a patch in RG |
bc225e3
to
83f7fb3
Compare
0b915df
to
3a89de5
Compare
Thanks for help guys. I have my tests passing now, but this implementation is adding 4-6 minutes to bundler test suit (has redudant connection checking). We can probably also use |
I'm so sorry I didn't follow up on the bundler tests troubleshooting, I forgot about this! Happily, @sonalkr132 found his way through and also documented it ❤️. |
Sorry for not following up, @sonalkr132. I'll have a look and see if I understand what's going on 👍 |
@sonalkr132 The test that are failing in your alternative patch seem to be failing only in CI because... they only run in CI, because of being slow. |
@bronzdoc I looked into this and you're right. I noticed this when reopening #2696 and CI passed (it should've failed when being tested against The problem is that currently we only test against a remote branch or tag, not against the checked out code of As per the lines of code in bundler's Rakefile that I pointed out earlier: https://github.com/bundler/bundler/blob/master/Rakefile#L9-L14, this seems unintentional, so I'll fix it to make our specs against bundler a proper regression run and not just a sanity check against master. |
I created rubygems/bundler#7070 to fix this. If it's correct, we can start specifying |
e1ecb35
to
3a89de5
Compare
hey, what's the status of this? It'd be awesome to have IPv4 fallback in place. |
Hi @sonalkr132 👋 I second the previous comment, the fallback to ipv4 would be very helpful. Would you have an idea on the next steps here? |
I'm able to reproduce the IPv6 error using my company VPN that is for some reason not able to resolve rubygems.org. Rubygems.org using IPv4 works though. So I can assist with the testing if that can help getting this out of the door. |
Hi, sorry about the delay in response here. Now we are using GitHub ci and I have been meaning to update this with a slightly less intrusive change (which could pass the test suit), however, haven't been able to find time for it. IPv6 not working for some users (particularly in Europe) is also a Fastly issue, where only sites hosted on Fastly are unreachable. We have filed a support ticket about this (so have other users) but no progress so far. |
Hello @sonalkr132! Today I spent some time getting a ipv6 tunnel (ipv6 adoption in Spain is not really a thing) so that I can have ipv6, and then breaking it through Then I rebased & installed this PR and timeouts were fixed 👍. I also tried some Can you share the realworld bundler stuff that was not playing well with this? |
3a89de5
to
62ee8d5
Compare
ruby/net-http#10 seems a very good idea to me, since it also fixes very noticiable inefficiencies in Anyways, yeah, I saw the comment that |
ruby/net-http#10 was merged by @hsbt, so the currently proposed ruby-core implementation of happy eyeballs (which was also just updated to address feedback) might be enough now. |
My IPv6 connectivity broke again today, which gave me the opportunity to test this out, Initially:
I added
I removed
So it looks like this works as intended! |
Nice!!! |
Should we move this forward then @sonalkr132? |
Yeah, I think so. This is disabled by default, so it can hardly do any harm. We can document it as a workaround for now and eventually migrate to fix in ruby/ruby#4038, which is much more comprehensive. I am not sure if we would ever enable current flag by default. @ddfreyne please do open a ticket on https://support.fastly.com and add support@rubygems.org or my email in cc. Make sure that your connection issue is only for fastly IPs or any IPv6 address.
|
Sadly the IPv6 problem lies entirely on my ISP’s side, not Fastly’s. I get an IPv6 address, which works intermittently: occasionally all IPv6 traffic, whether to Google or Fastly or anything else, gets dropped and thus yields timeouts. |
Can you rebase it @sonalkr132? |
Multiple users have reported that IPv6 address of rubygems.org is unreachable[1][2]. We have verified using online tools[3][4] as well as all of aws regions with ipv6 support that there is no misconfig issue on our part. IPv6 route being broken is a bit expected and hence, RFC8305[5] recommends starting connection to both IPv4 and IPv6 address in parallel and use whichever returns first (this is TLDR explanation, check RFC for details). This patch of TCPSocket.initialize does something similar, as in, it starts connection to all resolved IPs in parallel and uses first successful connection's IP to initalize the TCPSocket. We use `Net:HTTP` as our remote featcher and it internally uses TCPSocket.open[6]. Even tho support of IPv6 fallback is spotty, I have open a ruby issue for it[7]. [1] https://help.rubygems.org/discussions/problems/33671-suggestion-for-easy-solution-for-the-ipv6-related-timeout-problem [2] https://help.rubygems.org/discussions/problems/31074-timeout-error [3] https://www.ssllabs.com/ssltest/analyze.html?d=rubygems.org [4] https://ready.chair6.net/?url=rubygems.org [5] https://tools.ietf.org/html/rfc8305 [6] https://github.com/ruby/ruby/blob/4444025d16ae1a586eee6a0ac9bdd09e33833f3c/lib/net/http.rb#L950 [7] https://bugs.ruby-lang.org/issues/15628
use `export IPV4_FALLBACK_ENABLED=true` to enable
81cf0e3
to
3f8e070
Compare
rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm merging this after CI is it looks ok to you @sonalkr132.
yes, please go ahead. should we add a guide/blog for this? seems like it can use a page for context around the issue in Net:HTTP and error message. |
Which version will this fall into? |
@akostadinov most likely 3.2.11 |
Yes, I'll release it tomorrow hopefully. And yes, a guide/blog would be cool, an easy place to link to so we don't need to repeat the same instructions again and again when people run into this. |
Fallback to IPv4 when IPv6 is unreachable (cherry picked from commit 533a242)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be better to fix net/http to use Socket.connect_nonblock
than to spawn all these threads?
It sounds like you want to report an issue/suggestion to https://github.com/ruby/net-http? |
@deivid-rodriguez I was asking a question. |
Oh, sorry, I didn't mean to be rude. I don't know the answer to your question. The current implementation seems to be working just fine, and we might not even need to enable this by default at all, since it seems like it will be fixed in ruby. But your question seems like it could lead to an enhancement to |
Description:
Multiple users have reported that IPv6 address of rubygems.org is
unreachable[1][2]. We have verified using online tools[3][4] as well
as all of aws regions with ipv6 support that there is no misconfig issue
on our part.
IPv6 route being broken is a bit expected and hence, RFC8305[5]
recommends starting connection to both IPv4 and IPv6 address in parallel
and use whichever returns first (this is TLDR explanation, check RFC for
details). This patch of TCPSocket.initialize does something similar, as
in, it starts connection to all resolved IPs in parallel and uses first
successful connection's IP to initalize the TCPSocket. We use
Net:HTTP
as our remote featcher and it internally uses TCPSocket.open[6].
Even tho support of IPv6 fallback is spotty, I have open a ruby issue
for it[7].
About test for this.. we would need to simulate a ipv6 broken route, only way I know of is using
ip6tables
and that needssudo
.Fixes #3641.
Closes #3463.
Tasks:
I will abide by the code of conduct.