Skip to content

Commit

Permalink
db: add Options.WithFSDefaults
Browse files Browse the repository at this point in the history
Add a facility for easily layering in the default VFS middleware—currently the
disk-health checking FS. Options.EnsureDefaults by default uses the disk-health
checking FS, but most of our tests explicitly set a VFS, and in particular a
*vfs.MemFS. These tests have always run without the disk-health checking
filesystem layer. Use the new WithFSDefaults method across many Pebble unit
tests and the Pebble metamorphic tests.

This is sufficient to surface the concurrent `Sync` operations observed in
cockroachdb/cockroach#96422 and cockroachdb/cockroach#96414. See cockroachdb#2282 for
context on where this panic is originating.
  • Loading branch information
jbowens committed Feb 6, 2023
1 parent 10f3aff commit cc2a8bc
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 12 deletions.
4 changes: 2 additions & 2 deletions cleaner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ func TestArchiveCleaner(t *testing.T) {

var buf syncedBuffer
mem := vfs.NewMem()
opts := &Options{
opts := (&Options{
Cleaner: ArchiveCleaner{},
FS: loggingFS{mem, &buf},
WALDir: "wal",
}
}).WithFSDefaults()

datadriven.RunTest(t, "testdata/cleaner", func(td *datadriven.TestData) string {
switch td.Cmd {
Expand Down
5 changes: 3 additions & 2 deletions internal/metamorphic/meta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ func testMetaRun(t *testing.T, runDir string, seed uint64, historyPath string) {
opts.FS = vfs.NewMem()
}
}
opts.WithFSDefaults()

dir := opts.FS.PathJoin(runDir, "data")
// Set up the initial database state if configured to start from a non-empty
Expand Down Expand Up @@ -238,7 +239,7 @@ func readHistory(t *testing.T, historyPath string) []string {
// subsequent invocation will overwrite that output. A test can be re-run by
// using the `--run-dir` flag. For example:
//
// go test -v -run TestMeta --run-dir _meta/standard-017
// go test -v -run TestMeta --run-dir _meta/standard-017
//
// This will reuse the existing operations present in _meta/ops, rather than
// generating a new set.
Expand All @@ -247,7 +248,7 @@ func readHistory(t *testing.T, historyPath string) []string {
// pseudorandom number generator seed. If a failure occurs, the seed is
// printed, and the full suite of tests may be re-run using the `--seed` flag:
//
// go test -v -run TestMeta --seed 1594395154492165000
// go test -v -run TestMeta --seed 1594395154492165000
//
// This will generate a new `_meta/<test>` directory, with the same operations
// and options. This must be run on the same commit SHA as the original
Expand Down
26 changes: 18 additions & 8 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func (o *IterOptions) getLogger() Logger {
// Specifically, when configured with a RangeKeyMasking.Suffix _s_, and there
// exists a range key with suffix _r_ covering a point key with suffix _p_, and
//
// _s_ ≤ _r_ < _p_
// _s_ ≤ _r_ < _p_
//
// then the point key is elided.
//
Expand Down Expand Up @@ -848,13 +848,7 @@ func (o *Options) EnsureDefaults() *Options {
}

if o.FS == nil {
o.FS, o.private.fsCloser = vfs.WithDiskHealthChecks(vfs.Default, 5*time.Second,
func(name string, duration time.Duration) {
o.EventListener.DiskSlow(DiskSlowInfo{
Path: name,
Duration: duration,
})
})
o.WithFSDefaults()
}
if o.FlushSplitBytes <= 0 {
o.FlushSplitBytes = 2 * o.Levels[0].TargetFileSize
Expand All @@ -879,6 +873,22 @@ func (o *Options) EnsureDefaults() *Options {
return o
}

// WithFSDefaults configures the Options to wrap the configured filesystem with
// the default virtual file system middleware, like disk-health checking.
func (o *Options) WithFSDefaults() *Options {
if o.FS == nil {
o.FS = vfs.Default
}
o.FS, o.private.fsCloser = vfs.WithDiskHealthChecks(o.FS, 5*time.Second,
func(name string, duration time.Duration) {
o.EventListener.DiskSlow(DiskSlowInfo{
Path: name,
Duration: duration,
})
})
return o
}

func (o *Options) equal() Equal {
if o.Comparer.Equal == nil {
return bytes.Equal
Expand Down

0 comments on commit cc2a8bc

Please sign in to comment.