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: "client connection lost" in TestTransportPingWhenReading #42514

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

x/net/http2: "client connection lost" in TestTransportPingWhenReading #42514

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

Comments

@bcmills
Copy link
Contributor

bcmills commented Nov 11, 2020

2020-11-10T03:11:24-69a7880/aix-ppc64
2020-11-10T03:11:24-69a7880/darwin-amd64-10_15
2020-11-10T03:11:24-69a7880/linux-arm
2020-11-10T03:11:24-69a7880/linux-ppc64-buildlet
2020-11-09T17:26:40-a11eb1b/aix-ppc64
2020-11-09T17:26:40-a11eb1b/darwin-amd64-10_15
2020-11-09T17:26:40-a11eb1b/freebsd-arm64-dmgk
2020-11-09T17:26:40-a11eb1b/linux-ppc64-buildlet
2020-10-31T05:49:03-ff519b6/linux-ppc64-buildlet
2020-10-31T05:49:03-ff519b6/solaris-amd64-oraclerel
2020-10-29T22:17:08-28c70e6/linux-ppc64-buildlet
2020-10-29T22:17:08-28c70e6/linux-ppc64le-buildlet
2020-10-27T13:37:19-8eef523/linux-ppc64-buildlet
2020-10-26T09:15:29-146b70c/dragonfly-amd64-5_8
2020-10-24T04:28:10-be3efd7/dragonfly-amd64-5_8
2020-10-22T23:12:55-08b3837/dragonfly-amd64
2020-10-22T23:12:55-08b3837/solaris-amd64-oraclerel
2020-10-21T03:54:29-f585440/linux-ppc64le-buildlet
2020-10-20T06:53:57-d65d470/plan9-amd64-9front
2020-10-16T16:51:38-7b1cca2/dragonfly-amd64
2020-10-16T16:51:38-7b1cca2/freebsd-arm64-dmgk
2020-10-10T22:47:23-4f7140c/plan9-386-0intro
2020-10-08T22:28:04-59f7323/freebsd-arm-paulzhol
2020-10-06T15:34:59-a7d1128/freebsd-arm-paulzhol
2020-10-02T20:24:02-0a1ea39/android-386-emu
2020-09-25T08:00:53-05aa5d4/freebsd-arm-paulzhol
2020-09-23T18:22:12-328152d/aix-ppc64
2020-09-23T18:22:12-328152d/freebsd-arm-paulzhol
2020-09-04T19:48:48-62affa3/freebsd-arm-paulzhol
2020-09-04T19:48:48-62affa3/linux-ppc64-buildlet
2020-09-04T19:48:48-62affa3/plan9-386-0intro
2020-08-13T13:45:08-3edf25e/freebsd-arm-paulzhol
2020-08-13T13:45:08-3edf25e/linux-ppc64-buildlet
2020-06-25T00:16:55-4c52546/freebsd-arm64-dmgk
2020-06-02T11:40:24-627f964/dragonfly-amd64-5_8
2020-06-02T11:40:24-627f964/linux-ppc64-buildlet
2020-05-28T22:51:25-3c3fba1/linux-ppc64-buildlet
2020-05-20T18:23:14-0ba52f6/darwin-amd64-10_12

CC @fraenkel @neild @tombergan @empijei @bradfitz
@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 11, 2020
@fraenkel
Copy link
Contributor

I am thinking that if these are slower machines, we should:

  1. not run these tests in parallel
  2. allow for more variance on expected timings

The tests that are failing are given 200-300ms to succeed which is probably too small if we have 4 tests running concurrently and slower hardware.

@bcmills
Copy link
Contributor Author

bcmills commented Nov 12, 2020

Yeah, in general tests should not make any assumptions about things taking a “reasonable” amount of time.

(t.Deadline can be useful if you need to report some extra detail in case of a failure, but otherwise the test should generally keep running until it sees the events it expected to see, regardless of how long they take.)

@gopherbot
Copy link

Change https://golang.org/cl/269797 mentions this issue: http2: rework Ping test to rely less on timing

@tklauser
Copy link
Member

I've seen this occur again on x/net tip in a TryBot run for https://golang.org/cl/304330 on the android-amd64-emu builder: https://storage.googleapis.com/go-build-log/2e944012/android-amd64-emu_22945a4b.log

@bcmills bcmills mentioned this issue Sep 28, 2021
Closed
@golang golang locked and limited conversation to collaborators Mar 24, 2022
dteh pushed a commit to dteh/fhttp that referenced this issue Jun 22, 2022
@fraenkel
http2: rework Ping test to rely less on timing
eab6698 
Pings are either expected to occur, so count until you reach your goal
before a deadline, or they do not occur, and the deadline is exceeded.

Fixes golang/go#42514

Change-Id: If9ff19ed4954bee83ddeba83a4ac9c2d43f6e1c1
Reviewed-on: https://go-review.googlesource.com/c/net/+/269797
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
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
No milestone
Development

No branches or pull requests
4 participants
@tklauser @fraenkel @bcmills @gopherbot