-
Notifications
You must be signed in to change notification settings - Fork 17.5k
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/netutil: TestLimitListener failures with "Client.Timeout exceeded while awaiting headers" #22926
Comments
A recent x/net build failure has occurred a couple of times recently, but doesn't appear to be new. See an older one here https://build.golang.org/log/149eb7b73916520a758e84d70bc3c3ec9da16705 The output is similar to the one bradfitz included above, but lacks the file descriptor mention. Unclear if they are the exact same issue.
|
Change https://golang.org/cl/369054 mentions this issue: |
This may be a platform bug, but nobody has investigated it since the issue was filed in 2017. Skip it to reduce noise from flakes. For golang/go#22926 Change-Id: I8f310a539992c4552abb50fea3c7adc2149818ef Reviewed-on: https://go-review.googlesource.com/c/net/+/369054 Trust: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: David du Colombier <0intro@gmail.com> TryBot-Result: Gopher Robot <gobot@golang.org>
Curiously, the failures on non-plan9 builders have returned!
2021-12-15T06:06:38-4ddde0e-de690c2/windows-arm64-10 |
Change https://golang.org/cl/372495 mentions this issue: |
This is a release-blocker via #11811. (It also has a fix CL pending.) |
Change https://golang.org/cl/372714 mentions this issue: |
Still needs some tuning to work around #50216, and to make the tests a bit more robust. |
In CL 372495 I cleaned up TestLimitListener so that it would not fail spuriously. However, upon further thought I realized that the original test was actually checking two different properties (steady-state saturation, and actual overload), and the cleaned-up test was only checking one of those (overload). This change adds a separate test for steady-state saturation, and makes the overload test more robust to spurious connections (which could occur, for example, if another test running on the machine accidentally dials this test's open port). The test cleanup also revealed a bad interaction with an existing bug in the js/wasm net.TCPListener implementation (filed as golang/go#50216), for which I have added a workaround in (*limitListener).Accept. For golang/go#22926 Change-Id: I727050a8254f527c7455de296ed3525b6dc90141 Reviewed-on: https://go-review.googlesource.com/c/net/+/372714 Trust: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Should be fixed as of CL 372714. |
Change https://golang.org/cl/380155 mentions this issue: |
…il to dial When the listener saturates, the kernel will typically buffer the remaining pending connections (so they will eventually succeed in dialing). However, that behavior is not guaranteed, and it empirically doesn't always hold: a failure was observed in https://build.golang.org/log/5ac7312814bcff4841563be043f28aaefa9b3c90. We do expect to be able to dial at least as many connections as the listener will accept, and we expect every connection that is accepted to be served to completion. Updates golang/go#22926 Change-Id: I4cb39c8f39fda0dcb905f548612ccdf1856f2a66 Reviewed-on: https://go-review.googlesource.com/c/net/+/380155 Trust: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
TestLimitListener had made a lot of assumptions about the kernel's willingness to queue unaccepted connections, and relied on arbitrary timeouts to shed load if the queue saturates. This change eliminates the arbitrary timeouts, replacing them with synchronization and cancellation and leaving only a couple of arbitrary sleeps (that can be exceeded by arbitrary amounts without causing the test to fail). Fixes golang/go#22926 Change-Id: Ibecff6254ec966e1cc98cf96c71493f18d3aaebe Reviewed-on: https://go-review.googlesource.com/c/net/+/372495 Trust: Bryan Mills <bcmills@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
In CL 372495 I cleaned up TestLimitListener so that it would not fail spuriously. However, upon further thought I realized that the original test was actually checking two different properties (steady-state saturation, and actual overload), and the cleaned-up test was only checking one of those (overload). This change adds a separate test for steady-state saturation, and makes the overload test more robust to spurious connections (which could occur, for example, if another test running on the machine accidentally dials this test's open port). The test cleanup also revealed a bad interaction with an existing bug in the js/wasm net.TCPListener implementation (filed as golang/go#50216), for which I have added a workaround in (*limitListener).Accept. For golang/go#22926 Change-Id: I727050a8254f527c7455de296ed3525b6dc90141 Reviewed-on: https://go-review.googlesource.com/c/net/+/372714 Trust: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
…il to dial When the listener saturates, the kernel will typically buffer the remaining pending connections (so they will eventually succeed in dialing). However, that behavior is not guaranteed, and it empirically doesn't always hold: a failure was observed in https://build.golang.org/log/5ac7312814bcff4841563be043f28aaefa9b3c90. We do expect to be able to dial at least as many connections as the listener will accept, and we expect every connection that is accepted to be served to completion. Updates golang/go#22926 Change-Id: I4cb39c8f39fda0dcb905f548612ccdf1856f2a66 Reviewed-on: https://go-review.googlesource.com/c/net/+/380155 Trust: Bryan Mills <bcmills@google.com> Run-TryBot: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
https://build.golang.org/log/b5964d8ee324ab1c9a0e36fd018bebe61a5ae09f
Out of fds? Timing/cancelation issues? @0intro?
The text was updated successfully, but these errors were encountered: