net/http: add MaxConnLifespan to Transport

[josephcopenhaver]

Existing implementation appeared to have no way to set a connection
max lifetime. A max lifetime allows for refreshing connection lifecycle
concerns such as dns resolutions for those that need it.

When initialized to a non-zero value the connection will be closed after
the duration has passed. This change is backwards compatible.

Fixes #23427

The only other knob I would like to add to the default transport is an assurance that at most MaxConns # of connections will be opened but I think I can use the round-tripper interface to add this knob as long as the net.Conn interface is always guaranteed to have Close() called.

[google-cla]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[google-cla]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[josephcopenhaver]

@googlebot I signed it!

[gopherbot]

This PR (HEAD: 4902316) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/327474 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Go Bot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://golang.org/doc/contribute.html#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://golang.org/s/release
for more details.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/327474.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

This PR (HEAD: fea56c1) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/327474 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: 60f9d00) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/327474 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: b78ef9a) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/327474 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: 88e5505) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/327474 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: 28c82af) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/327474 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: 984dc1b) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/327474 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: 7cc745d) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/327474 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: e637fbf) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/327474 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Damien Neil:

Patch Set 9:

(1 comment)

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/327474.
After addressing review feedback, remember to publish your drafts!

[leilei911]

Hi @josephcopenhaver, thanks for creating the PR. I was wondering when you think we could merge the PR. We have a DNS resolution issue with our service and could really use your fix.

[gopherbot]

This PR (HEAD: a881869) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/327474 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[josephcopenhaver]

@leilei911 I am a new contributor to go's standard sdk in general and unfortunately I do not have the influence to provide you with a timeline.

The latest comment from @neild at https://go-review.googlesource.com/c/go/+/327474 indicates that Any new package API needs to go through the proposal process. which shows as resolved and may be in an upcoming candidate patch set if I read this all correctly.

Edit:

I kinda expected more conversation around how to couple such a feature to DNS TTLs a client discovers in the natural flow of things without taking such a simple approach. However most places where go is used at scale can operate just fine with a client override that does not try to couple itself with a DNS request's response. This solves real problems with balancing now where people only have service discovery, so I am for it! Also there is the concern of some hosts being just raw IP addresses where such a thing like DNS does not even come into play, so it does have value if someone uses a non dns backed discovery method.

If the maintainers provide feedback that this feature request should be smarter in any way ( such as allow the value to change over time ), I am open to making improvements. Right now I'm just keeping the patch up up to date with the origin and the ball is in their court unless I have misread anything. :-)

[josephcopenhaver]

Cross posting from: #23427 (comment)

I wonder how much of this problem is purely one of loadbalancing concepts people may be trying to implement.

If a host record resolves to 3 ip nodes and suddenly we add or remove one, then respectively we need to ensure the new node starts taking requests as soon as we know about it so load spreads ( no need to close connections until we approach the max connection limit or idle timeouts happen ) or remove connections tied to the deregistering ( potentially already deregistered ) ip address.

The latter case of removing a node is trivial and fairly self-correcting before any DNS TTL and refresh cycle would inform us on average.

In the same token, idle connection timeout can create "randomly hot" nodes over time by just the probability of FIFO rotating through connections cyclicly. A max lifespan is a fairly nuclear solution to this problem when perhaps envoy proxy should be the full-featured solution for balancing out-of-app.

For people wanting a solution in the same binary they really probably want LIFO connection queuing under a round-robin ( or some distance/efficiency-aware weighted function ) connection broker over the LIFO queues partitioned by IP. This would allow for any "logically extra" connections the service can live without to end up dying off just by virtue of the connection timeouts.

If DNS appears to remove a host we can assume the DNS source has determined that the host should get phased out and schedule the relevant connections to die as soon as their transactions finish or immediately inform the broker to kill that pool of trusted connections.

As a DNS entry is discovered for a hostname a LIFO queue would be created and added to the connection broker. Load would spread almost immediately across the upstream offering, and old connections no longer getting any use on the other nodes would start to time out and die off, expensive new connection flows continue to be avoided.

Connection max lifetime/reuse is likely not the feature people need or want. ( If you do need/want it cool, my PR offers it! ) It just looks like what we're reaching for in this conversation when we really want to observe+schedule DNS resolution and use that to inform a connection pooling strategy where we can change the implementation of popping and pushing to a queue we maintain externally that is LIFO or FIFO or anything else - but note we need to know the connection's target IP to achieve these goals. It's worth discussing in more detail by persons probably smarter than myself. Typically when users of one of my libs want agency I will happily give them the simplest interface they need to achieve their goals and default the behavior to off. Perhaps the maintainers are willing to do similar here?