Repository: cockroachdb/pebble
Pull request #542 (2020-02-17 23:12:56)

internal/metamorphic: duo of ingestion improvements
* internal/metamorphic: add option to use DB.Apply for DB.Ingest operations
* internal/metamorphic: randomly choose [1,3] batches to ingest


Patch:
diff --git a/internal/metamorphic/generator.go b/internal/metamorphic/generator.go
index 4525f300..6861b7a8 100644
--- a/internal/metamorphic/generator.go
+++ b/internal/metamorphic/generator.go
@@ -522,12 +522,19 @@ func (g *generator) writerIngest() {
 		return
 	}
 
-	// TODO(peter): randomly choose N batches to ingest.
-	batchID := g.liveBatches.rand(g.rng)
-	g.batchClose(batchID)
+	// Ingest between 1 and 3 batches.
+	batchIDs := make([]objID, 0, 1+g.rng.Intn(3))
+	for i := 0; i < cap(batchIDs); i++ {
+		batchID := g.liveBatches.rand(g.rng)
+		g.batchClose(batchID)
+		batchIDs = append(batchIDs, batchID)
+		if len(g.liveBatches) == 0 {
+			break
+		}
+	}
 
 	g.add(&ingestOp{
-		batchIDs: []objID{batchID},
+		batchIDs: batchIDs,
 	})
 }
 
diff --git a/internal/metamorphic/ops.go b/internal/metamorphic/ops.go
index 46ed9410..54cb1480 100644
--- a/internal/metamorphic/ops.go
+++ b/internal/metamorphic/ops.go
@@ -194,6 +194,20 @@ type ingestOp struct {
 }
 
 func (o *ingestOp) run(t *test, h *history) {
+	// We can only use apply as an alternative for ingestion if we are ingesting
+	// a single batch. If we are ingesting multiple batches, the batches may
+	// overlap which would cause ingestion to fail but apply would succeed.
+	if t.testOpts.ingestUsingApply && len(o.batchIDs) == 1 {
+		id := o.batchIDs[0]
+		b := t.getBatch(id)
+		w := t.getWriter(makeObjID(dbTag, 0))
+		err := w.Apply(b, t.writeOpts)
+		_ = b.Close()
+		t.clearObj(id)
+		h.Recordf("%s // %v", o, err)
+		return
+	}
+
 	var paths []string
 	var err error
 	for i, id := range o.batchIDs {
diff --git a/internal/metamorphic/options.go b/internal/metamorphic/options.go
index 06932838..34cc73d8 100644
--- a/internal/metamorphic/options.go
+++ b/internal/metamorphic/options.go
@@ -35,6 +35,9 @@ func parseOptions(opts *testOptions, data string) error {
 			case "TestOptions.strictfs":
 				opts.strictFS = true
 				return true
+			case "TestOptions.ingest_using_apply":
+				opts.ingestUsingApply = true
+				return true
 			default:
 				return false
 			}
@@ -46,11 +49,14 @@ func parseOptions(opts *testOptions, data string) error {
 
 func optionsToString(opts *testOptions) string {
 	str := opts.opts.String()
+	if opts.strictFS || opts.ingestUsingApply {
+		str += "\n[TestOptions]\n"
+	}
 	if opts.strictFS {
-		str += `
-[TestOptions]
-  strictfs=true
-`
+		str += "  strictfs=true\n"
+	}
+	if opts.ingestUsingApply {
+		str += "  ingest_using_apply=true\n"
 	}
 	return str
 }
@@ -70,6 +76,8 @@ func defaultOptions() *pebble.Options {
 type testOptions struct {
 	opts     *pebble.Options
 	strictFS bool
+	// Use Batch.Apply rather than DB.Ingest.
+	ingestUsingApply bool
 }
 
 func standardOptions() []*testOptions {
@@ -148,6 +156,10 @@ func standardOptions() []*testOptions {
   bytes_per_sync=1073741824
 [TestOptions]
   strictfs=true
+`,
+		18: `
+[TestOptions]
+  ingest_using_apply=true
 `,
 	}
 
@@ -192,5 +204,6 @@ func randomOptions(rng *rand.Rand) *testOptions {
 	if testOpts.strictFS {
 		opts.DisableWAL = false
 	}
+	testOpts.ingestUsingApply = rng.Intn(2) != 0
 	return testOpts
 }


Test patch:
diff --git a/internal/metamorphic/meta_test.go b/internal/metamorphic/meta_test.go
index b0463a6d..3befcf3f 100644
--- a/internal/metamorphic/meta_test.go
+++ b/internal/metamorphic/meta_test.go
@@ -27,16 +27,10 @@ import (
 
 // TODO(peter):
 //
-// Verify the metamorphic test catchs various bugs:
-// - Instability of keys returned from range-del-iters and used by truncate
-// - Lack of support for lower/upper bound in flushableBatchIter
-//
 // Miscellaneous:
 // - Add support for different comparers. In particular, allow reverse
 //   comparers and a comparer which supports Comparer.Split (by splitting off
 //   a variable length suffix).
-// - Ingest and Apply can be randomly swapped leading to testing of
-//   interesting cases.
 // - DeleteRange can be used to replace Delete, stressing the DeleteRange
 //   implementation.
 // - Add support for Writer.LogData