net/http: WriteTimeout not reset in http2 #18437

pnelson · 2016-12-27T20:07:30Z

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

$ go version
go version go1.8beta2 linux/amd64

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

Ubuntu 16.04 amd64

$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/pnelson"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build506631447=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

I wrote a simple HTTP/2 server with a WriteTimeout set to reproduce.

https://github.com/pnelson/go-timeouts-test

Using a load testing tool for 5 seconds yields latencies to the point of client timeout. Also reproducible in browser by refreshing the page a few times.

What did you expect to see?

Request latency to be well under 2s.

What did you see instead?

Request latency greater than 30s (client timeout) after a few requests from the same client, presumably due to connection reuse and timeout not being reset.

This sounds a bit similar to an older issue with ReadTimeout, #16450.

The text was updated successfully, but these errors were encountered:

vcabbage · 2016-12-27T20:18:43Z

@pnelson and I chatted about this issue on Slack. As far as I can tell, HTTP1 write deadlines are reset here, but since HTTP2 takes a different path the deadline is never reset.

The patch below resolved the issue for me. (It may well not be the correct fix, but I think it narrows down the issue.)

index 2e0b3c905a..c9aa12948f 100644
--- a/src/net/http/h2_bundle.go
+++ b/src/net/http/h2_bundle.go
@@ -4323,6 +4323,9 @@ func (sc *http2serverConn) processHeaders(f *http2MetaHeadersFrame) error {
        if sc.hs.ReadTimeout != 0 {
                sc.conn.SetReadDeadline(time.Time{})
        }
+       if sc.hs.WriteTimeout != 0 {
+               sc.conn.SetWriteDeadline(time.Now().Add(sc.hs.WriteTimeout))
+       }

        go sc.runHandler(rw, req, handler)
        return nil

gopherbot · 2016-12-29T04:15:23Z

CL https://golang.org/cl/34723 mentions this issue.

gopherbot · 2016-12-29T21:30:35Z

CL https://golang.org/cl/34724 mentions this issue.

Current handling of WriteTimeout for http2 does not extend the timeout on new streams. Disable the WriteTimeout in http2 for 1.8 release. Fixes test added in https://golang.org/cl/34723 Updates golang/go#18437 Change-Id: I366899fb4ff2e740610cad71e004141d092699a2 Reviewed-on: https://go-review.googlesource.com/34724 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>

Current handling of WriteTimeout for http2 does not extend the timeout on new streams. Disable the WriteTimeout in http2 for 1.8 release. Updates #18437 Change-Id: I20480432ab176f115464434645defb56ebeb6ece Reviewed-on: https://go-review.googlesource.com/34723 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>

gopherbot · 2016-12-30T01:55:23Z

CL https://golang.org/cl/34726 mentions this issue.

gopherbot · 2016-12-30T01:55:33Z

CL https://golang.org/cl/34727 mentions this issue.

gopherbot · 2016-12-31T18:40:24Z

CL https://golang.org/cl/34729 mentions this issue.

Updates http2 to x/net/http2 git rev 8fd7f25 for: http2: clear WriteTimeout in Server https://golang.org/cl/34724 And un-skip the new test. (The new test is a slow test, anyway, so won't affect builders or all.bash, but I verified it now passes.) Updates #18437 Change-Id: Ia91ae702edfd23747a9d6b61da284a5a957bfed3 Reviewed-on: https://go-review.googlesource.com/34729 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com> Reviewed-by: Kale B <kale@lemnisys.com> TryBot-Result: Gobot Gobot <gobot@golang.org>

rsc · 2017-01-04T17:01:48Z

@bradfitz, should this be closed?

bradfitz · 2017-01-04T17:10:42Z

No, it should be fixed properly later. There's a pending CL for 1.9 to respect the field. Currently the submitted workaround just makes the field ignored for 1.8.

Specifying timeouts when using HTTP/2 seems to cause reused connections to sometimes stay "Pending" for a long, long time -- this is bad for clients. Try again once 1.8 is fully released.

Updates #18437 Change-Id: Iaa8a35d18eca8be24763dd151ad9e324ecbf7f7b Reviewed-on: https://go-review.googlesource.com/34726 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

If the writeDeadline elapses RST_STREAM is sent with ErrCodeInternal. Fixes tests added in https://golang.org/cl/34726 Updates golang/go#18437 (fixes once it's bundled into net/http) Change-Id: I84e4f76753963c29cd3c06730d6bfbb7f1ee6051 Reviewed-on: https://go-review.googlesource.com/34727 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>

Updates golang#18437 Change-Id: Iaa8a35d18eca8be24763dd151ad9e324ecbf7f7b Reviewed-on: https://go-review.googlesource.com/34726 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

gopherbot · 2017-04-25T20:45:00Z

CL https://golang.org/cl/41753 mentions this issue.

Current handling of WriteTimeout for http2 does not extend the timeout on new streams. Disable the WriteTimeout in http2 for 1.8 release. Fixes test added in https://golang.org/cl/34723 Updates golang/go#18437 Change-Id: I366899fb4ff2e740610cad71e004141d092699a2 Reviewed-on: https://go-review.googlesource.com/34724 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>

If the writeDeadline elapses RST_STREAM is sent with ErrCodeInternal. Fixes tests added in https://golang.org/cl/34726 Updates golang/go#18437 (fixes once it's bundled into net/http) Change-Id: I84e4f76753963c29cd3c06730d6bfbb7f1ee6051 Reviewed-on: https://go-review.googlesource.com/34727 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>

This updates the bundled http2 package from git rev 5602c733f70afc6dcec6766be0d5034d4c4f14de of the x/net repo for: http2: Use NO_ERROR instead of CANCEL when responding before the request is finished https://golang.org/cl/40630 http2: enforce write deadline per stream https://golang.org/cl/34727 Updates golang/go#19948 Fixes golang/go#18437 Change-Id: I14500476e91551fa8f27a1aeb8ae3cac9600b74c Reviewed-on: https://go-review.googlesource.com/41753 Reviewed-by: Kale Blankenship <kale@lemnisys.com> Reviewed-by: Tom Bergan <tombergan@google.com> Run-TryBot: Kale Blankenship <kale@lemnisys.com> TryBot-Result: Gobot Gobot <gobot@golang.org>

rsc added this to the Go1.9 milestone Jan 4, 2017

rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 4, 2017

branlwyd referenced this issue in branlwyd/www-go Jan 5, 2017

Remove specified timeouts for now.

77d6bb4

Specifying timeouts when using HTTP/2 seems to cause reused connections to sometimes stay "Pending" for a long, long time -- this is bad for clients. Try again once 1.8 is fully released.

pwaller mentioned this issue Feb 10, 2017

http2: read timeout applies to TCP connection and not requests; interrupts valid requests caddyserver/caddy#1422

Closed

This was referenced Feb 21, 2017

Server sent events does not work with proxy caddyserver/caddy#1412

Closed

Default timeouts are quite low. caddyserver/caddy#1464

Closed

gopherbot closed this as completed in 0d3143e Apr 25, 2017

golang locked and limited conversation to collaborators Apr 25, 2018

gopherbot added the FrozenDueToAge label Apr 25, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

net/http: WriteTimeout not reset in http2 #18437

net/http: WriteTimeout not reset in http2 #18437

pnelson commented Dec 27, 2016

vcabbage commented Dec 27, 2016

gopherbot commented Dec 29, 2016

gopherbot commented Dec 29, 2016

gopherbot commented Dec 30, 2016

gopherbot commented Dec 30, 2016

gopherbot commented Dec 31, 2016

rsc commented Jan 4, 2017

bradfitz commented Jan 4, 2017

gopherbot commented Apr 25, 2017

net/http: WriteTimeout not reset in http2 #18437

net/http: WriteTimeout not reset in http2 #18437

Comments

pnelson commented Dec 27, 2016

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

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

What did you do?

What did you expect to see?

What did you see instead?

vcabbage commented Dec 27, 2016

gopherbot commented Dec 29, 2016

gopherbot commented Dec 29, 2016

gopherbot commented Dec 30, 2016

gopherbot commented Dec 30, 2016

gopherbot commented Dec 31, 2016

rsc commented Jan 4, 2017

bradfitz commented Jan 4, 2017

gopherbot commented Apr 25, 2017

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

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