Skip to content

Commit

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

Fix #96635.

Epic: None
Release note: None
  • Loading branch information
jbowens committed Feb 6, 2023
1 parent e75ede3 commit 27df3cb
Showing 1 changed file with 17 additions and 2 deletions.
19 changes: 17 additions & 2 deletions pkg/storage/pebble.go
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,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 @@ -1006,7 +1007,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) })
}
el := pebble.TeeEventListener(
p.makeMetricEtcEventListener(ctx),
Expand Down Expand Up @@ -1087,6 +1088,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 @@ -1132,7 +1144,7 @@ func (p *Pebble) makeMetricEtcEventListener(ctx context.Context) pebble.EventLis

log.Fatalf(ctx, "file write stall detected: %s", info)
} else {
go log.Errorf(ctx, "file write stall detected: %s", info)
p.async(func() { log.Errorf(ctx, "file write stall detected: %s", info) })
}
return
}
Expand Down Expand Up @@ -1172,6 +1184,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 27df3cb

Please sign in to comment.