net: Test{Read,Write}TimeoutFluctuation failures with "took over 1s" #36108
Comments
bcmills
added
the
NeedsInvestigation
bcmills
changed the title
bcmills
changed the title
bcmills
changed the title
|
2021-11-18T06:05:29-f6647f2/freebsd-arm-paulzhol |
|
2021-11-03T00:07:03-d6f7203/dragonfly-amd64 |
|
Part of the problem here, I think, is that the timing check relies on the runtime scheduler rather than the monotonic timer, and relies on the goroutine scheduler rather than comparing times: the timer in the test function that fires at the expected deadline and the deadline computed in the It also doesn't help that the test is marked as I think this test can fairly easily be made both much faster and much more robust. |
|
Change https://golang.org/cl/365334 mentions this issue: |
bcmills
added
the
NeedsFix
gopherbot
removed
the
NeedsInvestigation
|
Needs some fine-tuning: apparently 10% slop is not enough for OpenBSD even with a generous 512ms timeout. 😩 |
|
Change https://golang.org/cl/366176 mentions this issue: |
|
Change https://golang.org/cl/372215 mentions this issue: |
…penBSD Decrease the slop everywhere else, since NetBSD and OpenBSD seem to be the only ones that miss by that much. For #50189 Updates #36108 Change-Id: I22ac39cc7c254e40358fcd933b5a6016629602c3 Reviewed-on: https://go-review.googlesource.com/c/go/+/372215 Trust: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
Change https://go.dev/cl/415234 mentions this issue: |
This applies the net package CL 365334, CL 366176, CL 372215 to the os
package.
CL 365334:
These tests were checking for fairly narrow timing windows, but were
running in parallel and heavily dependent on timer and goroutine
scheduling. This change eliminates unnecessary goroutines, runs the
tests sequentially (dramatically shortening the timeouts to reduce the
penalty of doing so), and uses timestamp comparison instead of
background timers to hopefully gain some robustness from monotonic
timestamps.
Many of the other tests from this package would benefit from similar
simplifications, which we can apply if and when we notice flaky
failures or want to improve the latency of running the test.
CL 366176:
It appears that at least the OpenBSD kernel gets sloppier the longer
the timeout we give it, up to an observed overhead of around 25%.
Let's give it a little more than that (33%) in the comparison, and
also increase the growth curve to match the actual observed times
instead of exponential initial growth.
CL 372215:
Decrease the slop everywhere else, since NetBSD and OpenBSD seem to be
the only ones that miss by that much.
For #36108
For #50189
Fixes #50725 (we hope)
Change-Id: I0854d27af67ca9fcf0f9d9e4ff67acff4c2effc8
Reviewed-on: https://go-review.googlesource.com/c/go/+/415234
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
This applies the net package CL 365334, CL 366176, CL 372215 to the os
package.
CL 365334:
These tests were checking for fairly narrow timing windows, but were
running in parallel and heavily dependent on timer and goroutine
scheduling. This change eliminates unnecessary goroutines, runs the
tests sequentially (dramatically shortening the timeouts to reduce the
penalty of doing so), and uses timestamp comparison instead of
background timers to hopefully gain some robustness from monotonic
timestamps.
Many of the other tests from this package would benefit from similar
simplifications, which we can apply if and when we notice flaky
failures or want to improve the latency of running the test.
CL 366176:
It appears that at least the OpenBSD kernel gets sloppier the longer
the timeout we give it, up to an observed overhead of around 25%.
Let's give it a little more than that (33%) in the comparison, and
also increase the growth curve to match the actual observed times
instead of exponential initial growth.
CL 372215:
Decrease the slop everywhere else, since NetBSD and OpenBSD seem to be
the only ones that miss by that much.
For golang#36108
For golang#50189
Fixes golang#50725 (we hope)
Change-Id: I0854d27af67ca9fcf0f9d9e4ff67acff4c2effc8
Reviewed-on: https://go-review.googlesource.com/c/go/+/415234
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
2019-12-11T19:51:34-1b1fbb3/solaris-amd64-oraclerel
2019-10-21T16:42:10-7a6da21/solaris-amd64-oraclerel
2019-05-07T18:42:17-dc0388c/solaris-amd64-smartosbuildlet
See also #20838, #10775.
CC @bradfitz @ianlancetaylor
The text was updated successfully, but these errors were encountered: