From affd954686276d0beceb69267c64be00632814cf Mon Sep 17 00:00:00 2001 From: Jackson Owens Date: Wed, 11 Oct 2023 15:36:50 -0400 Subject: [PATCH] vfs/errorfs: add facilities for error injection in datadriven tests We frequently use the errorfs package to inject errors into filesystem operations to test error code paths. This commit builds off this package to support encoding error injection conditions within datadriven tests through parsing a small DSL. Future work will build off this to test handling of I/O errors during iteration. Informs #1115. Informs #2994. --- compaction_test.go | 2 +- data_test.go | 68 +++++++++++++ error_test.go | 4 +- ingest_test.go | 4 +- iterator_histories_test.go | 78 ++++----------- sstable/reader_test.go | 2 +- testdata/iter_histories/errors | 59 +++++++++++ vfs/errorfs/errorfs.go | 177 +++++++++++++++++++++++++++++++-- vfs/errorfs/errorfs_test.go | 44 ++++++++ 9 files changed, 368 insertions(+), 70 deletions(-) create mode 100644 testdata/iter_histories/errors create mode 100644 vfs/errorfs/errorfs_test.go diff --git a/compaction_test.go b/compaction_test.go index 4ea8595037..35905d3aa1 100644 --- a/compaction_test.go +++ b/compaction_test.go @@ -2898,7 +2898,7 @@ func TestCompactionErrorCleanup(t *testing.T) { ) mem := vfs.NewMem() - ii := errorfs.OnIndex(math.MaxInt32) // start disabled + ii := errorfs.OnIndex(math.MaxInt32, errorfs.Always()) // start disabled opts := (&Options{ FS: errorfs.Wrap(mem, ii), Levels: make([]LevelOptions, numLevels), diff --git a/data_test.go b/data_test.go index fa1cc1ae26..fb37cb2722 100644 --- a/data_test.go +++ b/data_test.go @@ -18,6 +18,7 @@ import ( "github.com/cockroachdb/datadriven" "github.com/cockroachdb/errors" + "github.com/cockroachdb/pebble/bloom" "github.com/cockroachdb/pebble/internal/base" "github.com/cockroachdb/pebble/internal/humanize" "github.com/cockroachdb/pebble/internal/keyspan" @@ -29,6 +30,7 @@ import ( "github.com/cockroachdb/pebble/objstorage/remote" "github.com/cockroachdb/pebble/sstable" "github.com/cockroachdb/pebble/vfs" + "github.com/cockroachdb/pebble/vfs/errorfs" "github.com/stretchr/testify/require" ) @@ -1447,3 +1449,69 @@ func runLSMCmd(td *datadriven.TestData, d *DB) string { } return d.mu.versions.currentVersion().String() } + +func parseDBOptionsArgs(opts *Options, args []datadriven.CmdArg) error { + for _, cmdArg := range args { + switch cmdArg.Key { + case "inject-errors": + injs := make([]errorfs.Injector, len(cmdArg.Vals)) + for i := 0; i < len(cmdArg.Vals); i++ { + inj, err := errorfs.ParseInjectorFromDSL(cmdArg.Vals[i]) + if err != nil { + return err + } + injs[i] = inj + } + opts.FS = errorfs.Wrap(opts.FS, errorfs.Any(injs...)) + case "format-major-version": + v, err := strconv.Atoi(cmdArg.Vals[0]) + if err != nil { + return err + } + // Override the DB version. + opts.FormatMajorVersion = FormatMajorVersion(v) + case "block-size": + v, err := strconv.Atoi(cmdArg.Vals[0]) + if err != nil { + return err + } + for i := range opts.Levels { + opts.Levels[i].BlockSize = v + } + case "index-block-size": + v, err := strconv.Atoi(cmdArg.Vals[0]) + if err != nil { + return err + } + for i := range opts.Levels { + opts.Levels[i].IndexBlockSize = v + } + case "target-file-size": + v, err := strconv.Atoi(cmdArg.Vals[0]) + if err != nil { + return err + } + for i := range opts.Levels { + opts.Levels[i].TargetFileSize = int64(v) + } + case "bloom-bits-per-key": + v, err := strconv.Atoi(cmdArg.Vals[0]) + if err != nil { + return err + } + fp := bloom.FilterPolicy(v) + opts.Filters = map[string]FilterPolicy{fp.Name(): fp} + for i := range opts.Levels { + opts.Levels[i].FilterPolicy = fp + } + case "merger": + switch cmdArg.Vals[0] { + case "appender": + opts.Merger = base.DefaultMerger + default: + return errors.Newf("unrecognized Merger %q\n", cmdArg.Vals[0]) + } + } + } + return nil +} diff --git a/error_test.go b/error_test.go index 68f546f5fe..cbd3c72e44 100644 --- a/error_test.go +++ b/error_test.go @@ -136,7 +136,7 @@ func TestErrors(t *testing.T) { errorCounts := make(map[string]int) for i := int32(0); ; i++ { - fs := errorfs.Wrap(vfs.NewMem(), errorfs.OnIndex(i)) + fs := errorfs.Wrap(vfs.NewMem(), errorfs.OnIndex(i, errorfs.Always())) err := run(fs) if err == nil { t.Logf("success %d\n", i) @@ -166,7 +166,7 @@ func TestErrors(t *testing.T) { func TestRequireReadError(t *testing.T) { run := func(formatVersion FormatMajorVersion, index int32) (err error) { // Perform setup with error injection disabled as it involves writes/background ops. - inj := errorfs.OnIndex(-1) + inj := errorfs.OnIndex(-1, errorfs.Always()) fs := errorfs.Wrap(vfs.NewMem(), inj) opts := &Options{ FS: fs, diff --git a/ingest_test.go b/ingest_test.go index 6cfc9659b1..2f0fb7f1a6 100644 --- a/ingest_test.go +++ b/ingest_test.go @@ -386,7 +386,7 @@ func TestIngestLinkFallback(t *testing.T) { src, err := mem.Create("source") require.NoError(t, err) - opts := &Options{FS: errorfs.Wrap(mem, errorfs.OnIndex(1))} + opts := &Options{FS: errorfs.Wrap(mem, errorfs.OnIndex(1, errorfs.Always()))} opts.EnsureDefaults().WithFSDefaults() objSettings := objstorageprovider.DefaultSettings(opts.FS, "") // Prevent the provider from listing the dir (where we may get an injected error). @@ -2182,7 +2182,7 @@ func TestIngestError(t *testing.T) { require.NoError(t, w.Set([]byte("d"), nil)) require.NoError(t, w.Close()) - inj := errorfs.OnIndex(-1) + inj := errorfs.OnIndex(-1, errorfs.Always()) d, err := Open("", &Options{ FS: errorfs.Wrap(mem, inj), Logger: panicLogger{}, diff --git a/iterator_histories_test.go b/iterator_histories_test.go index bd178b2def..5f958ec2c9 100644 --- a/iterator_histories_test.go +++ b/iterator_histories_test.go @@ -13,8 +13,6 @@ import ( "github.com/cockroachdb/datadriven" "github.com/cockroachdb/errors" - "github.com/cockroachdb/pebble/bloom" - "github.com/cockroachdb/pebble/internal/base" "github.com/cockroachdb/pebble/internal/invariants" "github.com/cockroachdb/pebble/internal/testkeys" "github.com/cockroachdb/pebble/sstable" @@ -41,8 +39,9 @@ func TestIterHistories(t *testing.T) { iters[name] = it return it } + var opts *Options parseOpts := func(td *datadriven.TestData) (*Options, error) { - opts := &Options{ + opts = &Options{ FS: vfs.NewMem(), Comparer: testkeys.Comparer, FormatMajorVersion: FormatRangeKeys, @@ -50,61 +49,12 @@ func TestIterHistories(t *testing.T) { sstable.NewTestKeysBlockPropertyCollector, }, } + opts.DisableAutomaticCompactions = true opts.EnsureDefaults() opts.WithFSDefaults() - - for _, cmdArg := range td.CmdArgs { - switch cmdArg.Key { - case "format-major-version": - v, err := strconv.Atoi(cmdArg.Vals[0]) - if err != nil { - return nil, err - } - // Override the DB version. - opts.FormatMajorVersion = FormatMajorVersion(v) - case "block-size": - v, err := strconv.Atoi(cmdArg.Vals[0]) - if err != nil { - return nil, err - } - for i := range opts.Levels { - opts.Levels[i].BlockSize = v - } - case "index-block-size": - v, err := strconv.Atoi(cmdArg.Vals[0]) - if err != nil { - return nil, err - } - for i := range opts.Levels { - opts.Levels[i].IndexBlockSize = v - } - case "target-file-size": - v, err := strconv.Atoi(cmdArg.Vals[0]) - if err != nil { - return nil, err - } - for i := range opts.Levels { - opts.Levels[i].TargetFileSize = int64(v) - } - case "bloom-bits-per-key": - v, err := strconv.Atoi(cmdArg.Vals[0]) - if err != nil { - return nil, err - } - fp := bloom.FilterPolicy(v) - opts.Filters = map[string]FilterPolicy{fp.Name(): fp} - for i := range opts.Levels { - opts.Levels[i].FilterPolicy = fp - } - case "merger": - switch cmdArg.Vals[0] { - case "appender": - opts.Merger = base.DefaultMerger - default: - return nil, errors.Newf("unrecognized Merger %q\n", cmdArg.Vals[0]) - } - } + if err := parseDBOptionsArgs(opts, td.CmdArgs); err != nil { + return nil, err } return opts, nil } @@ -128,10 +78,11 @@ func TestIterHistories(t *testing.T) { datadriven.RunTest(t, path, func(t *testing.T, td *datadriven.TestData) string { switch td.Cmd { case "define": + var err error if err := cleanup(); err != nil { return err.Error() } - opts, err := parseOpts(td) + opts, err = parseOpts(td) if err != nil { return err.Error() } @@ -140,12 +91,23 @@ func TestIterHistories(t *testing.T) { return err.Error() } return runLSMCmd(td, d) - + case "reopen": + var err error + if err := cleanup(); err != nil { + return err.Error() + } + if err := parseDBOptionsArgs(opts, td.CmdArgs); err != nil { + return err.Error() + } + d, err = Open("", opts) + require.NoError(t, err) + return "" case "reset": + var err error if err := cleanup(); err != nil { return err.Error() } - opts, err := parseOpts(td) + opts, err = parseOpts(td) if err != nil { return err.Error() } diff --git a/sstable/reader_test.go b/sstable/reader_test.go index 4549094789..bd80bf7ccc 100644 --- a/sstable/reader_test.go +++ b/sstable/reader_test.go @@ -647,7 +647,7 @@ func TestInjectedErrors(t *testing.T) { f, err := vfs.Default.Open(filepath.FromSlash(prebuiltSST)) require.NoError(t, err) - r, err := newReader(errorfs.WrapFile(f, errorfs.OnIndex(int32(i))), ReaderOptions{}) + r, err := newReader(errorfs.WrapFile(f, errorfs.OnIndex(int32(i), errorfs.Always())), ReaderOptions{}) if err != nil { return firstError(err, f.Close()) } diff --git a/testdata/iter_histories/errors b/testdata/iter_histories/errors new file mode 100644 index 0000000000..a95d3f2dbd --- /dev/null +++ b/testdata/iter_histories/errors @@ -0,0 +1,59 @@ +reset +---- + +batch commit +set a a +set b b +set c c +set d d +---- +committed 4 keys + +# Scan forward + +combined-iter +seek-ge a +next +next +next +next +---- +a: (a, .) +b: (b, .) +c: (c, .) +d: (d, .) +. + +reopen +---- + +combined-iter +first +next +next +next +next +---- +a: (a, .) +b: (b, .) +c: (c, .) +d: (d, .) +. + +reopen inject-errors=(reads(pathMatch("*.sst", onIndex(4, always)))) +---- + +combined-iter +first +first +next +next +next +next +---- +err=pebble: backing file 000004 error: injected error +a: (a, .) +b: (b, .) +c: (c, .) +d: (d, .) +. diff --git a/vfs/errorfs/errorfs.go b/vfs/errorfs/errorfs.go index 9ebc45a913..c828af1dfe 100644 --- a/vfs/errorfs/errorfs.go +++ b/vfs/errorfs/errorfs.go @@ -6,9 +6,14 @@ package errorfs import ( "fmt" + "go/scanner" + "go/token" "io" "math/rand" "os" + "path/filepath" + "strconv" + "strings" "sync" "sync/atomic" "time" @@ -97,8 +102,8 @@ const ( // OnIndex constructs an injector that returns an error on // the (n+1)-th invocation of its MaybeError function. It // may be passed to Wrap to inject an error into an FS. -func OnIndex(index int32) *InjectIndex { - ii := &InjectIndex{} +func OnIndex(index int32, next Injector) *InjectIndex { + ii := &InjectIndex{next: next} ii.index.Store(index) return ii } @@ -106,6 +111,7 @@ func OnIndex(index int32) *InjectIndex { // InjectIndex implements Injector, injecting an error at a specific index. type InjectIndex struct { index atomic.Int32 + next Injector } // Index returns the index at which the error will be injected. @@ -115,11 +121,11 @@ func (ii *InjectIndex) Index() int32 { return ii.index.Load() } func (ii *InjectIndex) SetIndex(v int32) { ii.index.Store(v) } // MaybeError implements the Injector interface. -func (ii *InjectIndex) MaybeError(_ Op, _ string) error { - if ii.index.Add(-1) == -1 { - return errors.WithStack(ErrInjected) +func (ii *InjectIndex) MaybeError(op Op, path string) error { + if ii.index.Add(-1) != -1 { + return nil } - return nil + return ii.next.MaybeError(op, path) } // WithProbability returns a function that returns an error with the provided @@ -155,6 +161,165 @@ type Injector interface { MaybeError(op Op, path string) error } +// Always returns an injector that always injects an error. +func Always() Injector { return InjectorFunc(func(Op, string) error { return ErrInjected }) } + +// Any returns an injector that injects an error if any the provided injectors +// inject an error. +func Any(injectors ...Injector) Injector { + return InjectorFunc(func(op Op, path string) error { + for _, inj := range injectors { + if err := inj.MaybeError(op, path); err != nil { + return err + } + } + return nil + }) +} + +// PathMatch returns an injector that injects an error on file paths that +// match the provided pattern (according to filepath.Match) and for which the +// provided next injector injects an error. +func PathMatch(pattern string, next Injector) Injector { + return InjectorFunc(func(op Op, path string) error { + if matched, err := filepath.Match(pattern, path); err != nil { + // Only possible error is ErrBadPattern, indicating an issue with + // the test itself. + panic(err) + } else if matched { + return next.MaybeError(op, path) + } + return nil + }) +} + +// ParseInjectorFromDSL parses a string encoding a ruleset describing when +// errors should be injected. There are a handful of supported functions and +// primitives: +// - "always" is a primitive that injects an error every time +// - "any(injector, [injector]...)" injects an error if any of the provided +// injectors inject an error +// - "pathMatch(pattern, injector)" injects an error if an operation's file path +// matches the provided shell pattern and the provided injector injects an +// error. +// - "onIndex(idx, injector)" injects an error on the idx-th operation if the +// provided injector injects an error. +// - "reads(injector)" injects an error on all read operations for which +// the provided injector injects an error. +// - "writes(injector)" injects an error on all write operations for which +// the provided injector injects an error. +// +// Example: pathMatch("*.sst", onIndex(5, always)) is a rule set that will +// inject an error on the 5-th I/O operation involving an sstable. +func ParseInjectorFromDSL(d string) (inj Injector, err error) { + defer func() { + if r := recover(); r != nil { + var ok bool + err, ok = r.(error) + if !ok { + panic(r) + } + } + }() + + fset := token.NewFileSet() + file := fset.AddFile("", -1, len(d)) + var s scanner.Scanner + s.Init(file, []byte(strings.TrimSpace(d)), nil /* no error handler */, 0) + inj = parseInjectorDSLFunc(&s) + consumeTok(&s, token.SEMICOLON) + consumeTok(&s, token.EOF) + return inj, err +} + +var dslParsers map[string]func(*scanner.Scanner) Injector + +func init() { + dslParsers = map[string]func(*scanner.Scanner) Injector{ + "always": func(*scanner.Scanner) Injector { return Always() }, + "any": func(s *scanner.Scanner) Injector { + var injs []Injector + consumeTok(s, token.LPAREN) + injs = append(injs, parseInjectorDSLFunc(s)) + pos, tok, lit := s.Scan() + for tok == token.COMMA { + injs = append(injs, parseInjectorDSLFunc(s)) + pos, tok, lit = s.Scan() + } + if tok != token.RPAREN { + panic(errors.Errorf("errorfs: unexpected token %s (%q) at %#v", tok, lit, pos)) + } + return Any(injs...) + }, + "pathMatch": func(s *scanner.Scanner) Injector { + consumeTok(s, token.LPAREN) + pattern := mustUnquote(consumeTok(s, token.STRING)) + consumeTok(s, token.COMMA) + next := parseInjectorDSLFunc(s) + consumeTok(s, token.RPAREN) + return PathMatch(pattern, next) + }, + "onIndex": func(s *scanner.Scanner) Injector { + consumeTok(s, token.LPAREN) + i, err := strconv.ParseInt(consumeTok(s, token.INT), 10, 32) + if err != nil { + panic(err) + } + consumeTok(s, token.COMMA) + next := parseInjectorDSLFunc(s) + consumeTok(s, token.RPAREN) + return OnIndex(int32(i), next) + }, + "reads": func(s *scanner.Scanner) Injector { + consumeTok(s, token.LPAREN) + next := parseInjectorDSLFunc(s) + consumeTok(s, token.RPAREN) + return InjectorFunc(func(o Op, path string) error { + if o.OpKind() == OpKindRead { + return next.MaybeError(o, path) + } + return nil + }) + }, + "writes": func(s *scanner.Scanner) Injector { + consumeTok(s, token.LPAREN) + next := parseInjectorDSLFunc(s) + consumeTok(s, token.RPAREN) + return InjectorFunc(func(o Op, path string) error { + if o.OpKind() == OpKindWrite { + return next.MaybeError(o, path) + } + return nil + }) + }, + } +} + +func parseInjectorDSLFunc(s *scanner.Scanner) Injector { + fn := consumeTok(s, token.IDENT) + p, ok := dslParsers[fn] + if !ok { + panic(errors.Errorf("errorfs: unknown func %q", fn)) + } + return p(s) +} + +func consumeTok(s *scanner.Scanner, expected token.Token) (lit string) { + pos, tok, lit := s.Scan() + if tok != expected { + panic(errors.Errorf("errorfs: unexpected token %s (%q) at %#v", tok, lit, pos)) + } + return lit +} + +func mustUnquote(lit string) string { + s, err := strconv.Unquote(lit) + if err != nil { + panic(errors.Newf("errorfs: unquoting %q: %v", lit, err)) + } + return s +} + // FS implements vfs.FS, injecting errors into // its operations. type FS struct { diff --git a/vfs/errorfs/errorfs_test.go b/vfs/errorfs/errorfs_test.go new file mode 100644 index 0000000000..bfa6b05554 --- /dev/null +++ b/vfs/errorfs/errorfs_test.go @@ -0,0 +1,44 @@ +// Copyright 2023 The LevelDB-Go and Pebble Authors. All rights reserved. Use +// of this source code is governed by a BSD-style license that can be found in +// the LICENSE file. + +package errorfs + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestValidDSL(t *testing.T) { + testCases := map[string]bool{ + `always`: true, + `alwoes`: false, + `always()`: false, + `pathMatch("foo/*.sst", always)`: true, + `pathMatch(foo/*.sst, always)`: false, + `pathMatch("foo/*.sst", alwoes)`: false, + `pathMatch("foo/*.sst", "", always)`: false, + `pathMatch(always, "foo/*.sst")`: false, + `onIndex(1, always)`: true, + `pathMatch("foo/bar/*.sst", onIndex(1, always))`: true, + `onIndex(always, "foo/*.sst")`: false, + `any(always, always, always)`: true, + `any(onIndex(2, always), pathMatch("*.sst", always))`: true, + `any(1, 4, 5)`: false, + `reads(pathMatch("*.sst", always))`: true, + `writes(pathMatch("*.sst", always))`: true, + `any(always, always, always`: false, + `onIndex(foo, always)`: false, + } + for dsl, ok := range testCases { + t.Run(dsl, func(t *testing.T) { + _, err := ParseInjectorFromDSL(dsl) + if ok { + require.NoError(t, err) + } else { + require.Error(t, err) + } + }) + } +}