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

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

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

Comments

@networkimprov
Copy link

@networkimprov networkimprov commented Nov 5, 2020

@fraenkel
Copy link
Contributor

@fraenkel fraenkel commented Nov 5, 2020

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

@bcmills
Copy link
Member Author

@bcmills 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 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

@gopherbot gopherbot commented Nov 6, 2020

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants