Skip to content

Commit

Permalink
storage/concurrency: add fixed delay in waitForAsyncGoroutinesToStall
Browse files Browse the repository at this point in the history
This prevents false detection of stalls in a few cases, like
when receiving on a buffered channel that already has an element
in it.

Before this, the first few iterations of `testutils.SucceedsSoon`
were so quick that `TestConcurrencyManagerBasic/basic` saw cases
where the wrong channel select was detected as a stall point. This
occurred ~1/1,000 times. With this change, the test has stood up
to over 10,000 iterations of stress.
  • Loading branch information
nvanbenschoten committed Feb 27, 2020
1 parent da4479d commit 53393f1
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions pkg/storage/concurrency/concurrency_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -667,15 +667,17 @@ func (m *monitor) hasNewEvents(g *monitoredGoroutine) bool {
// performs an action that may unblock any of the goroutines.
func (m *monitor) waitForAsyncGoroutinesToStall(t *testing.T) {
// Iterate until we see two iterations in a row that both observe all
// monitored goroutines to be stalled and also both observe the exact
// same goroutine state. Waiting for two iterations to be identical
// isn't required for correctness because the goroutine dump itself
// should provide a consistent snapshot of all goroutine states and
// statuses (runtime.Stack(all=true) stops the world), but it still
// seems like a generally good idea.
// monitored goroutines to be stalled and also both observe the exact same
// goroutine state. The goroutine dump provides a consistent snapshot of all
// goroutine states and statuses (runtime.Stack(all=true) stops the world).
var status []*stack.Goroutine
filter := funcName((*monitor).runAsync)
testutils.SucceedsSoon(t, func() error {
// Add a small fixed delay after each iteration. This is sufficient to
// prevents false detection of stalls in a few cases, like when
// receiving on a buffered channel that already has an element in it.
defer time.Sleep(1 * time.Millisecond)

prevStatus := status
status = goroutineStatus(t, filter, &m.buf)
if len(status) == 0 {
Expand Down

0 comments on commit 53393f1

Please sign in to comment.