Skip to content

Commit

Permalink
net/http: scale rstAvoidanceDelay to reduce test flakiness
Browse files Browse the repository at this point in the history
As far as I can tell, some flakiness is unavoidable in tests
that race a large client request write against a server's response
when the server doesn't read the full request.
It does not appear to be possible to simultaneously ensure that
well-behaved clients see EOF instead of ECONNRESET and also prevent
misbehaving clients from consuming arbitrary server resources.
(See RFC 7230 §6.6 for more detail.)

Since there doesn't appear to be a way to cleanly eliminate
this source of flakiness, we can instead work around it:
we can allow the test to adjust the hard-coded delay if it
sees a plausibly-related failure, so that the test can retry
with a longer delay.

As a nice side benefit, this also allows the tests to run more quickly
in the typical case: since the test will retry in case of spurious
failures, we can start with an aggressively short delay, and only back
off to a longer one if it is really needed on the specific machine
running the test.

Fixes #57084.
Fixes #51104.
For #58398.

Change-Id: Ia4050679f0777e5eeba7670307a77d93cfce856f
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest-race,gotip-linux-amd64-race,gotip-windows-amd64-race
Reviewed-on: https://go-review.googlesource.com/c/go/+/527196
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
  • Loading branch information
Bryan C. Mills authored and gopherbot committed Sep 13, 2023
1 parent 545e4f3 commit 6760f20
Show file tree
Hide file tree
Showing 4 changed files with 419 additions and 289 deletions.
18 changes: 18 additions & 0 deletions src/net/http/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,3 +315,21 @@ func ResponseWriterConnForTesting(w ResponseWriter) (c net.Conn, ok bool) {
}
return nil, false
}

func init() {
// Set the default rstAvoidanceDelay to the minimum possible value to shake
// out tests that unexpectedly depend on it. Such tests should use
// runTimeSensitiveTest and SetRSTAvoidanceDelay to explicitly raise the delay
// if needed.
rstAvoidanceDelay = 1 * time.Nanosecond
}

// SetRSTAvoidanceDelay sets how long we are willing to wait between calling
// CloseWrite on a connection and fully closing the connection.
func SetRSTAvoidanceDelay(t *testing.T, d time.Duration) {
prevDelay := rstAvoidanceDelay
t.Cleanup(func() {
rstAvoidanceDelay = prevDelay
})
rstAvoidanceDelay = d
}
Loading

0 comments on commit 6760f20

Please sign in to comment.