Skip to content

Commit

Permalink
replay: fix data race
Browse files Browse the repository at this point in the history
Fix a data race caused by accessing an atomic-only member variable without an
an atomic load. Rename the variable to clarify that it should be accessed
through atomics only.

Fix cockroachdb#2214.
  • Loading branch information
jbowens committed Jan 5, 2023
1 parent 10bdef4 commit bec1aa2
Showing 1 changed file with 9 additions and 6 deletions.
15 changes: 9 additions & 6 deletions replay/replay.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,12 @@ type Runner struct {
// It is guarded by the finishedCompactionNotifier condition variable lock.
compactionEndedCount int64

// compactionStartCount uses atomics to keep track of the previous cumulative
// compaction count.
compactionStartCount int64
// atomicCompactionStartCount keeps track of the cumulative number of
// compactions started. It's incremented by a pebble.EventListener's
// CompactionStart handler.
//
// Must be accessed with atomics.
atomicCompactionStartCount int64

// compactionsHaveQuiesced is a channel that is closed once compactions have
// quiesced allowing the compactionNotified goroutine to complete.
Expand Down Expand Up @@ -218,7 +221,7 @@ func (r *Runner) refreshMetrics(ctx context.Context) error {

fetchDoneState := func() {
r.finishedCompactionNotifier.L.Lock()
done = r.compactionEndedCount == atomic.LoadInt64(&r.compactionStartCount)
done = r.compactionEndedCount == atomic.LoadInt64(&r.atomicCompactionStartCount)
r.finishedCompactionNotifier.L.Unlock()
}

Expand Down Expand Up @@ -324,7 +327,7 @@ func (r *Runner) eventListener() pebble.EventListener {
uint64(time.Since(writeStallBegin).Nanoseconds()))
},
CompactionBegin: func(_ pebble.CompactionInfo) {
atomic.AddInt64(&r.compactionStartCount, 1)
atomic.AddInt64(&r.atomicCompactionStartCount, 1)
},
CompactionEnd: func(_ pebble.CompactionInfo) {
r.finishedCompactionNotifier.L.Lock()
Expand Down Expand Up @@ -558,7 +561,7 @@ func (r *Runner) prepareWorkloadSteps(ctx context.Context) error {
func (r *Runner) compactionNotified(ctx context.Context) error {
r.finishedCompactionNotifier.L.Lock()
for {
if r.compactionEndedCount < r.compactionStartCount {
if r.compactionEndedCount < atomic.LoadInt64(&r.atomicCompactionStartCount) {
r.finishedCompactionNotifier.Wait()
}
r.finishedCompactionNotifier.L.Unlock()
Expand Down

0 comments on commit bec1aa2

Please sign in to comment.