← Back to context

Comment by gigatexal

16 hours ago

Can you go more into this? I don’t quite follow

Go's http.Client will keepalive a TCP/TLS connection to save you handshake latency on second requests. But it can only do this if you completely finish reading the last request.

Now in 1.27:

> http.Response.Body drains itself on Close. For HTTP/1, closing the body now reads and discards any unread content (up to a conservative limit) so the connection can be reused. For most programs this is a transparent win [...]

Great, so i no longer have to io.Copy(io.Discard, resp.Body) in the err case, one less thing to worry about; but

> if you were leaning on an early Close to abort a large download, set Transport.DisableKeepAlives to opt out.

That's a subtle behaviour change. Any previous Go program which used Close in this way - say for an infinite event stream - now hangs, soaking up bandwidth.

In the past, the Go team have searched the entire Github corpus for misuse before making changes like this. I don't have a reference but I assume an appropriate level of consideration went into this decision.

EDIT: ""up to a conservative limit"" so this is not so bad after all.

  • Is “a conservative limit” a high limit or a low limit? If it is high such that many responses will still be drained it would keep reading those infinite streams for a long time. If it is low it might still not drain all normal sized messages.

    Anyway, this is why it pays off to read release notes closely and have a decent test suite.

    • The drain is asynchronous, so it won’t block. The limit is 256 KB and 50 ms.

Say you have some code that does a request to an HTTP/1 dependency, and if it get an error response, just closes the connection without reading the response body.

Go 1.26 in practice never re-used that connection, it always established a new one because you can't reuse a connection which has a pending response ready to be read.

Go 1.27 will now consume the body for you, causing your application to re-use connections much more aggressively, bringing in potential edge cases (e.g. dependency is broken, connection is now permanently unusable, your app no longer recovers automatically).

To be clear, I'm very glad for the change and I had equivalent code in our in-house framework to do just that, but yeah it does change the behavior in a way that it could expose undetected issues.