Skip to content

Commit

Permalink
vfs: handle concurrent directory Syncs in disk-health checking
Browse files Browse the repository at this point in the history
The file-level disk-health checker requires that a file not be used
concurrently, because it only supports timing a single in-flight operation at a
time. Pebble did not adhere to this contract for the data directory, which it
synced concurrently. This had the potential to leave a data directory Sync
untimed if an in-flight Syncs' timestamp was overwritten by the completion of
another Sync.

In #2282 we began checking for serialized writes in `invariants` builds. This
revealed these concurrent syncs in CockroachDB test failures under `-race`:
cockroachdb/cockroach#96414 and cockroachdb/cockroach#96422.
  • Loading branch information
jbowens committed Feb 2, 2023
1 parent 917d3f3 commit e9d3bb3
Showing 1 changed file with 25 additions and 5 deletions.
30 changes: 25 additions & 5 deletions vfs/disk_health.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,26 @@ func (d *diskHealthCheckingFile) timeDiskOp(opType OpType, op func()) {
op()
}

// diskHealthCheckingDir implements disk-health checking for directories. Unlike
// other files, we allow directories to receive concurrent write operations
// (Syncs are the only write operations supported by a directory.) Since the
// diskHealthCheckingFile's timeDiskOp can only track a single in-flight
// operation at a time, we time the operation using the filesystem-level
// timeFilesystemOp function instead.
type diskHealthCheckingDir struct {
File
name string
fs *diskHealthCheckingFS
}

// Sync implements the io.Syncer interface.
func (d *diskHealthCheckingDir) Sync() (err error) {
d.fs.timeFilesystemOp(d.name, OpTypeSync, func() {
err = d.File.Sync()
})
return err
}

// diskHealthCheckingFS adds disk-health checking facilities to a VFS.
// It times disk write operations in two ways:
//
Expand Down Expand Up @@ -576,11 +596,11 @@ func (d *diskHealthCheckingFS) OpenDir(name string) (File, error) {
}
// Directories opened with OpenDir must be opened with health checking,
// because they may be explicitly synced.
checkingFile := newDiskHealthCheckingFile(f, d.diskSlowThreshold, func(opType OpType, duration time.Duration) {
d.onSlowDisk(name, opType, duration)
})
checkingFile.startTicker()
return checkingFile, nil
return &diskHealthCheckingDir{
File: f,
name: name,
fs: d,
}, nil
}

// PathBase implements the FS interface.
Expand Down

0 comments on commit e9d3bb3

Please sign in to comment.