Skip to content

Commit

Permalink
inctimer: fix test flake where timer does not fire within time.
Browse files Browse the repository at this point in the history
[ upstream commit e695e48 ]

Running the test in a cpu constrained environment, such as:

```
docker run -v $(pwd):$(pwd) -w $(pwd) --cpus=0.1 -it golang:bullseye ./inctimer.test -test.v
```

I can fairly consistency reproduce a flake where the inctimer.After does not fire in time.
If I allow it to wait for an additional couple of ms, this seems to be sufficient to prevent failure.

It appears that goroutine scheduling latency can be significantly delayed in cpu restricted environments.
This seems unavoidable, so to fix the flake I'll allow the test to wait another 2ms to see if the inctimer eventually fires.

This will also log an error for delayed test fires, so if there is any other issues we can more easily debug them in the future.

Fixed: #25202

Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
  • Loading branch information
tommyp1ckles authored and YutaroHayakawa committed May 10, 2023
1 parent 9438350 commit 53088a5
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions pkg/inctimer/inctimer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,18 @@ func TestTimerHardReset(t *testing.T) {
tr, done := New()
defer done()
for i := 0; i < 100; i++ {
ch := tr.After(time.Millisecond)
select {
case <-tr.After(time.Millisecond):
case <-ch:
// Under CPU constrained environments, there may be a delay
// between the timer firing and the goroutine being scheduled,
case <-time.After(time.Millisecond * 2):
t.Fatal("`IncTimer`, after being reset, did not fire")
select {
case <-ch:
t.Log("Warning: `IncTimer` eventually fired, but was delayed (this is likely caused by constrained CPU resources and GC)")
case <-time.After(time.Millisecond * 2):
t.Fatal("`IncTimer` did not fire after being reset")
}
}
}
}

0 comments on commit 53088a5

Please sign in to comment.