rate: allow for more timing slop in TestWaitSimple
The 'd' constant is intentionally set fairly long to allow for builder
jitter; however, dFromDuration previously hard-coded only 1ms of
downward timing slop.
That slop can be introduced due to time spent between the call to
WaitN in the previous runWait and the call to time.Now in the current
runWait, and empirically may be a bit larger than 1ms on certain
builders (especially the android-amd64-emu builders, which also have
more clock drift than many other platforms; see golang/go#42513).
In addition, on some BSD platforms the slop in the upward direction
may actually be longer than d no matter how generously d is set. That
appears to be a platform bug (see golang/go#50189).
This change adjusts dFromDuration to round to the nearest d instead of
biasing in one direction or the other, and allows an additional factor
of slop on the affected BSD platforms.
Fixes #44067
Change-Id: Id4c073bee545be2291ad98158d764e19db0160cb
Reviewed-on: https://go-review.googlesource.com/c/time/+/383175
Trust: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Sameer Ajmani <sameer@golang.org>
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/rate/rate_test.go b/rate/rate_test.go
index 93a3378..1ca3711 100644
--- a/rate/rate_test.go
+++ b/rate/rate_test.go
@@ -233,11 +233,12 @@
ok bool
}
-// dFromDuration converts a duration to a multiple of the global constant d
+// dFromDuration converts a duration to the nearest multiple of the global constant d.
func dFromDuration(dur time.Duration) int {
- // Adding a millisecond to be swallowed by the integer division
- // because we don't care about small inaccuracies
- return int((dur + time.Millisecond) / d)
+ // Add d/2 to dur so that integer division will round to
+ // the nearest multiple instead of truncating.
+ // (We don't care about small inaccuracies.)
+ return int((dur + (d / 2)) / d)
}
// dSince returns multiples of d since t0
@@ -406,16 +407,45 @@
start := time.Now()
err := lim.WaitN(w.ctx, w.n)
delay := time.Since(start)
- if (w.nilErr && err != nil) || (!w.nilErr && err == nil) || w.delay != dFromDuration(delay) {
+
+ if (w.nilErr && err != nil) || (!w.nilErr && err == nil) || !waitDelayOk(w.delay, delay) {
errString := "<nil>"
if !w.nilErr {
errString = "<non-nil error>"
}
- t.Errorf("lim.WaitN(%v, lim, %v) = %v with delay %v ; want %v with delay %v",
- w.name, w.n, err, delay, errString, d*time.Duration(w.delay))
+ t.Errorf("lim.WaitN(%v, lim, %v) = %v with delay %v; want %v with delay %v (±%v)",
+ w.name, w.n, err, delay, errString, d*time.Duration(w.delay), d/2)
}
}
+// waitDelayOk reports whether a duration spent in WaitN is “close enough” to
+// wantD multiples of d, given scheduling slop.
+func waitDelayOk(wantD int, got time.Duration) bool {
+ gotD := dFromDuration(got)
+
+ // The actual time spent waiting will be REDUCED by the amount of time spent
+ // since the last call to the limiter. We expect the time in between calls to
+ // be executing simple, straight-line, non-blocking code, so it should reduce
+ // the wait time by no more than half a d, which would round to exactly wantD.
+ if gotD < wantD {
+ return false
+ }
+
+ // The actual time spend waiting will be INCREASED by the amount of scheduling
+ // slop in the platform's sleep syscall, plus the amount of time spent executing
+ // straight-line code before measuring the elapsed duration.
+ // (The latter is surely less than half a d.)
+ maxD := wantD
+ switch runtime.GOOS {
+ case "netbsd", "openbsd":
+ // NetBSD and OpenBSD tend to overshoot sleeps by a wide margin due to a
+ // suspected platform bug; see https://go.dev/issue/44067 and
+ // https://go.dev/issue/50189.
+ maxD = (wantD*3 + 1) / 2 // 150% of wantD, rounded up.
+ }
+ return gotD <= maxD
+}
+
func TestWaitSimple(t *testing.T) {
lim := NewLimiter(10, 3)
