Navigation Menu

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

Jump to bottom

net: enable happy eyeballs by default #22225

Closed
bobrik opened this issue Oct 12, 2017 · 12 comments
Closed

net: enable happy eyeballs by default #22225

bobrik opened this issue Oct 12, 2017 · 12 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal Proposal-Accepted
Milestone
Go1.12

Comments

@bobrik
Copy link

bobrik commented Oct 12, 2017

What version of Go are you using (go version)?

go version go1.9.1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

I suspect any OS, but here's what I'm using:

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ivan/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

Run the following program:

package main

import (
	"log"
	"net"
	"time"
)

func main() {
	d := net.Dialer{
		Timeout: 8 * time.Second,
	}

	s := time.Now()

	// IPv4 is example.com, IPv6 is a blackhole
	//
	// $ host broken-ipv6-working-ipv4.bobrik.name
	// broken-ipv6-working-ipv4.bobrik.name has address 93.184.216.34
	// broken-ipv6-working-ipv4.bobrik.name has IPv6 address 100::1
	c, err := d.Dial("tcp", "broken-ipv6-working-ipv4.bobrik.name:80")
	if err != nil {
		log.Fatal(err)
	}

	log.Printf("Dial to %v completed in %v", c.RemoteAddr(), time.Since(s))
}

What did you expect to see?

Connection succeeding to IPv4 address with no human perceptible delay, working IPv6 or not.

What did you see instead?

2017/10/12 02:39:42 Dial to 93.184.216.34:80 completed in 4.016407843s

This is the half of net.Dialer.Timeout.

What happened here is that my custom dialer did not set net.Dialer.DualStack to true explicitly.

Current docs: https://github.com/golang/go/blob/d2826d3e06/src/net/dial.go#L46-L51

// DualStack enables RFC 6555-compliant "Happy Eyeballs"
// dialing when the network is "tcp" and the host in the
// address parameter resolves to both IPv4 and IPv6 addresses.
// This allows a client to tolerate networks where one address
// family is silently broken.
DualStack bool

This setting is off by default in zero-value net.Dialer, net.Dial() and friends. However, it was turned on in http.DefaultTransport: #15324.

We fixed an issue in Grafana that caused 15s stalls for us:

Wikipedia says:

Implementations of Happy Eyeballs stacks exist in Google's Chrome web browser, Opera 12.10, Firefox version 13, OS X, and cURL.

I think Go should also have this on by default. This protects users from indefinite (half of default infinite connect timeout is still infinity) stalls if resolver returns both A and AAAA (or even AAAAA), but the network drops fancy IPv6 packets on the floor.

Current name DualStack is also slightly confusing, because Go dials both stacks in either case. Setting only controls Happy Eyeballs part, so having NoHappyEyeballs instead would allow zero-valued dialers to work seamlessly with broken IPv6.

If we end up not enabling Happy Eyeballs by default, this issue will at least hold the reason why.

cc @bradfitz
@ianlancetaylor ianlancetaylor changed the title Enable happy eyeballs by default net: enable happy eyeballs by default Oct 12, 2017
@ianlancetaylor ianlancetaylor added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 12, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Oct 12, 2017
@rsc
Copy link
Contributor

rsc commented Oct 23, 2017

If we change the default we want to make the zero Dialer pick up that default, because various code including package net does things like

 d := Dialer{Timeout: xxx}

to override one part of the default and expect the rest of the default behavior to carry forward. That would mean a second DisableDualStack bool and ignoring the current DualStack bool or something like that. Let's leave this API decision for Go 1.11, when @bradfitz and @mikioh can have more time to discuss it (Brad is away right now and will be until at least the freeze).

@rsc rsc modified the milestones: Go1.10, Go1.11 Oct 23, 2017
@bobrik
Copy link
Author

bobrik commented Oct 23, 2017

@rsc see the 2nd paragraph from the end in the original ticket description. I think we should go with DisableHappyEyeballs instead of DisableDualStack.

@bobrik
Copy link
Author

bobrik commented Feb 14, 2018

@bradfitz, @mikioh is this a better time when 1.11 window is open?

@mikioh
Copy link
Contributor

mikioh commented Feb 15, 2018 •
edited

I'm fine with having appropriate default values for stateful/connection-oriented communication setup using net.Dial and its variants.

Right now I have no concrete idea which variables should have default values but it seems like Timeout and DualStack might be mandatory. The most extreme case I've ever heard is some experiment using MPTCP over IP over MLPPP over some satellite communication protocol, but we can exclude such possibility.

Well, for naming thing, in early draft CLs the existing DualStack of Dialer was HappyEyeballs (or EnableHappyEyeballs) and @rsc proposed renaming; see What's next in networking? Joyful spleens? Ebullient elbows? Raving kneecaps perhaps.

@bobrik
Copy link
Author

bobrik commented Feb 21, 2018

I don't think that renaming RFC6555 is a good idea: https://tools.ietf.org/html/rfc6555

Timeouts seem like a whole big separate story, I'd rather focus this issue on Happy Eyeballs. Can we start by having DisableHappyEyeballs addition to the struct and deprecation of DualStack?

Suggested algorithm:

useHappyEyeballs := d.DualStack || !d.DisableHappyEyeballs
DisableHappyEyeballs DualStack Happy Eyeballs enabled
false false true (new default)
true false false (explicitly disabled)
false true true (explicitly enabled via DualStack)
true true false (explicitly weird)

The change here is that if user explicitly disables DualStack, they'll have to explicitly set DisableHappyEyeballs to true now to retain the same behavior.

The behavior is "connect to IPv6 and stall indefinitely if it doesn't respond". It's a fairly safe choice to assume this to be very exotic (if existing in practice at all) use case.

@mikioh
Copy link
Contributor

mikioh commented Feb 22, 2018 •
edited

IIUC, Eyeballs or Eyeball network is slang or a password in telco sector or some inner circle. I'm still not sure the reason why we need to stick with it, even if it's a standardized technology name, though I still don't have a good replacement for it.

If we need a new control knob that defaults to being enabled, it's better to name FeatureNameDisabled instead of DisableFeatureName.

Timeouts seem like a whole big separate story

I don't think so because in my understanding the purpose of Happy Eyeballs is just to establish a TCP connection in a reasonable period and to report an error as quickly as possible without having unreasonable long failover delays on funky circumstances. Timeouts in the go runtime (Dialer.Timeout, Dialer.FallbackDelay) and the kernel (max # of syn-rexmit * rexmit-delay) are part of the delays.

However, what you want here is just to run a connection setup race by default, that's probably fine.

@mikioh
Copy link
Contributor

mikioh commented Feb 22, 2018 •
edited

that's probably fine.

I mean, it's fine to send a CL containing:

@pavlo-v-chernykh
Copy link

Happy Eyeballs aka Fast Fallback. FastFallbackDisabled could be a good name for a var.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jun 27, 2018
@rsc
Copy link
Contributor

rsc commented Oct 3, 2018

It sounds like the plan needs to be:

  1. Deprecate (ignore) DualStack, because we want it to default to true and can't distinguish "unset" from "set to false explicitly".
  2. Default to happy eyeballs ON.
  3. Add new way to force eyeballs off.

One option is to add a new field for 3, but another option is to take the existing FallbackDelay field. If FallbackDelay < 0, then fallback is disabled.

Anyone object to that?

@rsc rsc changed the title net: enable happy eyeballs by default proposal: net: enable happy eyeballs by default Oct 3, 2018
@rsc
Copy link
Contributor

rsc commented Oct 10, 2018

No objections, and @bradfitz approves, so let's do this for Go 1.12 very soon and then we'll have 3 months before the release to undo this change if it causes problems.

@rsc rsc changed the title proposal: net: enable happy eyeballs by default net: enable happy eyeballs by default Oct 10, 2018
@bobrik
Copy link
Author

bobrik commented Oct 10, 2018

@rsc does my comment reflect your plan?

If so, I can probably carve some time to come up with a CL.

@arbll arbll mentioned this issue Oct 12, 2018
Merged
@gopherbot
Copy link

Change https://golang.org/cl/146659 mentions this issue: net: enable RFC 6555 Fast Fallback by default

@arbll arbll mentioned this issue Jun 4, 2019
Merged
@golang golang locked and limited conversation to collaborators Nov 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal Proposal-Accepted
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
7 participants
@bradfitz @bobrik @mikioh @rsc @pavlo-v-chernykh @ianlancetaylor @gopherbot