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: TestTransportGroupsPendingDials failures due to missing Close #52996

Open
bcmills opened this issue May 19, 2022 · 5 comments
Open

x/net/http2: TestTransportGroupsPendingDials failures due to missing Close #52996

bcmills opened this issue May 19, 2022 · 5 comments
Assignees
Labels
NeedsInvestigation
Milestone

Comments

@bcmills
Copy link
Member

@bcmills bcmills commented May 19, 2022

--- FAIL: TestTransportGroupsPendingDials (0.02s)
    transport_test.go:401: saw 0 closes; want 1
FAIL
FAIL	golang.org/x/net/http2	17.546s

greplogs -l -e 'FAIL: TestTransportGroupsPendingDials .*(?:\n[ ]{4}.*) saw 0 closes' --since=2021-01-01
2022-05-18T15:25:04-183a9ca-1f9f7db/linux-amd64-clang
2022-04-29T02:01:27-2871e0c-e7c56fe/freebsd-arm-paulzhol
2022-03-04T14:10:38-27dd868-c9b6063/freebsd-arm64-dmgk

This may be a symptom of #50027; see previously #43176.

(attn @neild @tombergan)

@bcmills bcmills added the NeedsInvestigation label May 19, 2022
@bcmills bcmills added this to the Go1.19 milestone May 19, 2022
@bcmills
Copy link
Member Author

@bcmills bcmills commented May 19, 2022

This failure mode has been observed on linux/amd64 (which is a first class port), so — especially since x/net/http2 is bundled into the standard library — this is a release-blocker for Go 1.19.

(If this isn't a priority to fix, a skip can be added to the test for this failure mode and then the issue can be moved to the backlog.)

@dmitshur dmitshur added the okay-after-beta1 label Jun 1, 2022
@neild neild self-assigned this Jun 8, 2022
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 8, 2022

Change https://go.dev/cl/411294 mentions this issue: http2: enable VerboseLogs in TestTransportGroupsPendingDials

@neild
Copy link
Contributor

@neild neild commented Jun 8, 2022

I'm not sure what's going on here.

This is not #50027: This test is expecting net.Conn.Close to have been called, which happens synchronously in CloseIdleConnections, not for the connection goroutines to have returned.

The only thing that makes sense is that a request has stayed live past Response.Body.Close being called (and is thus holding the connection live), which can happen in general, but I don't see how it happens in this test.

Flakes are super rare, and I haven't managed to reproduce it. Sent a CL to enable verbose HTTP/2 logging during this test; perhaps that'll point us at the problem the next time this flakes.

@neild
Copy link
Contributor

@neild neild commented Jun 8, 2022

Removing the release-blocker label: If there's a real underlying issue, it does not need to block the release. CL 411294 will hopefully let us collect more information towards resolving the flake.

@neild neild removed NeedsInvestigation release-blocker okay-after-beta1 labels Jun 8, 2022
@seankhliao seankhliao added the NeedsInvestigation label Jun 11, 2022
gopherbot pushed a commit to golang/net that referenced this issue Jun 17, 2022
This test is very, very rarely flaky. Enable additional logs to help
debug what's going on.

For golang/go#52996.

Change-Id: Ibccbfff94f51d2c813d48b48077537090bea4612
Reviewed-on: https://go-review.googlesource.com/c/net/+/411294
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jun 24, 2022

Rolling forward to 1.20. Please comment if you disagree. Thanks.

@ianlancetaylor ianlancetaylor removed this from the Go1.19 milestone Jun 24, 2022
@ianlancetaylor ianlancetaylor added this to the Go1.20 milestone Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
Status: No status
Development

No branches or pull requests

6 participants