Skip to content

Commit

Permalink
storage: fix TestPebbleMetricEventListener
Browse files Browse the repository at this point in the history
In cockroachdb#96096 handling of disk slow events was made asynchronous. This introduced a
race during Close where the goroutine logging could still be inflight.

Fix cockroachdb#96638.
Epic: None
Release note: None
  • Loading branch information
jbowens committed Feb 6, 2023
1 parent 94a8726 commit 40c5ba6
Showing 1 changed file with 20 additions and 3 deletions.
23 changes: 20 additions & 3 deletions pkg/storage/pebble.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,7 @@ type Pebble struct {
syncutil.Mutex
flushCompletedCallback func()
}
asyncDone sync.WaitGroup

// supportsRangeKeys is 1 if the database supports range keys. It must
// be accessed atomically.
Expand Down Expand Up @@ -943,7 +944,7 @@ func NewPebble(ctx context.Context, cfg PebbleConfig) (p *Pebble, err error) {
oldDiskSlow := lel.DiskSlow
lel.DiskSlow = func(info pebble.DiskSlowInfo) {
// Run oldDiskSlow asynchronously.
go oldDiskSlow(info)
p.async(func() { oldDiskSlow(info) })
}
cfg.Opts.EventListener = pebble.TeeEventListener(
p.makeMetricEtcEventListener(ctx),
Expand Down Expand Up @@ -992,6 +993,17 @@ func NewPebble(ctx context.Context, cfg PebbleConfig) (p *Pebble, err error) {
return p, nil
}

// async launches the provided function in a new goroutine. It uses a wait group
// to synchronize with (*Pebble).Close to ensure all launched goroutines have
// exited before Close returns.
func (p *Pebble) async(fn func()) {
p.asyncDone.Add(1)
go func() {
defer p.asyncDone.Done()
fn()
}()
}

func (p *Pebble) makeMetricEtcEventListener(ctx context.Context) pebble.EventListener {
return pebble.EventListener{
WriteStallBegin: func(info pebble.WriteStallBeginInfo) {
Expand Down Expand Up @@ -1042,8 +1054,10 @@ func (p *Pebble) makeMetricEtcEventListener(ctx context.Context) pebble.EventLis
log.Fatalf(ctx, "disk stall detected: pebble unable to write to %s in %.2f seconds",
info.Path, redact.Safe(info.Duration.Seconds()))
} else {
go log.Errorf(ctx, "disk stall detected: pebble unable to write to %s in %.2f seconds",
info.Path, redact.Safe(info.Duration.Seconds()))
p.async(func() {
log.Errorf(ctx, "disk stall detected: pebble unable to write to %s in %.2f seconds",
info.Path, redact.Safe(info.Duration.Seconds()))
})
}
return
}
Expand Down Expand Up @@ -1083,6 +1097,9 @@ func (p *Pebble) Close() {
}
p.closed = true

// Wait for any asynchronous goroutines to exit.
p.asyncDone.Wait()

handleErr := func(err error) {
if err == nil {
return
Expand Down

0 comments on commit 40c5ba6

Please sign in to comment.