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

x/net/http2: TestTransportRetryAfterGOAWAY failures on dragonfly and android builders #42381

Closed
bcmills opened this issue Nov 4, 2020 · 5 comments
Closed

x/net/http2: TestTransportRetryAfterGOAWAY failures on dragonfly and android builders #42381

bcmills opened this issue Nov 4, 2020 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@bcmills
Copy link
Contributor

bcmills commented Nov 4, 2020 

--- FAIL: TestTransportRetryAfterGOAWAY (1.02s)
    transport_test.go:3626: server1 got [FrameHeader HEADERS flags=END_STREAM|END_HEADERS stream=1 len=32]
    transport_test.go:3676: timed out
    transport_test.go:3650: server2 got [FrameHeader HEADERS flags=END_STREAM|END_HEADERS stream=1 len=32]
FAIL
FAIL	golang.org/x/net/http2	23.262s

2020-10-31T05:49:03-ff519b6/dragonfly-amd64
2020-10-31T05:49:03-ff519b6/dragonfly-amd64-5_8
2020-10-29T22:17:08-28c70e6/dragonfly-amd64
2020-10-20T06:53:57-d65d470/android-amd64-emu
2020-10-16T16:51:38-7b1cca2/dragonfly-amd64-5_8
2020-10-09T03:24:41-dbdefad/dragonfly-amd64-5_8
2020-09-30T14:50:03-4acb6c0/dragonfly-amd64
2020-09-27T03:25:02-5d4f700/dragonfly-amd64
2020-09-27T03:25:02-5d4f700/dragonfly-amd64-5_8
2020-09-04T19:48:48-62affa3/dragonfly-amd64-5_8
2020-08-22T12:43:28-c890458/dragonfly-amd64
2020-05-28T22:51:25-3c3fba1/dragonfly-amd64
2020-05-20T00:47:42-59133d7/dragonfly-amd64
2020-05-20T00:47:42-59133d7/dragonfly-amd64-5_8
2020-05-06T14:57:44-7e3656a/dragonfly-amd64-5_8
2020-04-25T23:01:54-ff2c4b7/android-386-emu
2019-12-06T10:30:17-1ddd1de/android-386-emu
2019-11-05T08:49:25-a882066/android-386-emu
2019-11-05T08:49:25-a882066/android-amd64-emu
2019-09-21T01:59:27-1a5e07d/android-386-emu
2019-08-27T16:04:01-ba9fcec/android-386-emu
2019-08-13T14:13:03-74dc4d7/android-amd64-emu

CC @neild @bradfitz @tombergan @empijei
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 4, 2020
@networkimprov
Copy link

cc @fraenkel

@fraenkel
Copy link
Contributor

fraenkel commented Nov 5, 2020

All we can do is bump the timeout in the test to deal with slower boxes.

@bcmills
Copy link
Contributor Author

bcmills commented Nov 5, 2020

Looks like that part of the test is here?
https://github.com/golang/net/blob/ff519b6c91021e6316e1df005bc19f266994ddda/http2/transport_test.go#L3669-L3678

Is the failure mode of this test a deadlock, or just a moderately-long delay? If the former, I'd be inclined to remove the select and time.After case entirely: if the test deadlocks, you ideally want a goroutine dump (which is exactly what a timed-out test would give you).

@fraenkel
Copy link
Contributor

fraenkel commented Nov 6, 2020

So its neither. All the test is doing is sending a single request, where the first server, responds with a GOAWAY and the client retries the request which goes to the second server that processes it.

Yes, we can ditch the timing aspect since its just there to bound how long 2 requests should take.

@gopherbot
Copy link

Change https://golang.org/cl/267977 mentions this issue: http2: remove the timeout since we don't know a good value

@dmitshur dmitshur added this to the Unreleased milestone Nov 6, 2020
@golang golang locked and limited conversation to collaborators Nov 10, 2021
dteh pushed a commit to dteh/fhttp that referenced this issue Jun 22, 2022
@fraenkel
http2: remove the timeout since we don't know a good value
5ce94a3 
The test is expected to finish so rather than wait on some arbitrary
timeout, let the go test timeout show us where things went wrong.

Fixes golang/go#42381

Change-Id: Ia9405f9f75b2f5f73ed3f8a540a74b7c64066ad0
Reviewed-on: https://go-review.googlesource.com/c/net/+/267977
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@networkimprov @fraenkel @dmitshur @bcmills @gopherbot