Repository: cockroachdb/pebble
Pull request #1249 (2021-09-01 16:17:33)

metamorphic: transform percentage of SINGLEDEL ops to DELETE ops
Generated SINGLEDEL operations are eligible for transformation into less
restrictive DELETE operations.

Transform a fixed percentage of SINGLEDEL operations at generation time
into DELETEs to further exercise the delete execution paths.

Related to cockroachdb/cockroach#69414.

cockroach-teamcity: 

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.png" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/cockroachdb/pebble/1249)
<!-- Reviewable:end -->


Patch:
diff --git a/internal/metamorphic/generator.go b/internal/metamorphic/generator.go
index 7d8cc5f8..8e1e6eb0 100644
--- a/internal/metamorphic/generator.go
+++ b/internal/metamorphic/generator.go
@@ -843,6 +843,12 @@ func (g *generator) writerSingleDelete() {
 	g.add(&singleDeleteOp{
 		writerID: writerID,
 		key:      key,
+		// Keys eligible for single deletes can be removed with a regular
+		// delete. Mutate a percentage of SINGLEDEL ops into DELETEs. Note that
+		// here we are only determining whether the replacement *could* happen.
+		// At test runtime, the `replaceSingleDelete` test option must also be
+		// set to true for the single delete to be replaced.
+		maybeReplaceDelete: g.rng.Float64() < 0.25,
 	})
 	g.tryRepositionBatchIters(writerID)
 }
diff --git a/internal/metamorphic/ops.go b/internal/metamorphic/ops.go
index 41aec405..a413ee84 100644
--- a/internal/metamorphic/ops.go
+++ b/internal/metamorphic/ops.go
@@ -132,18 +132,29 @@ func (o *deleteOp) String() string {
 
 // singleDeleteOp models a Write.SingleDelete operation.
 type singleDeleteOp struct {
-	writerID objID
-	key      []byte
+	writerID           objID
+	key                []byte
+	maybeReplaceDelete bool
 }
 
 func (o *singleDeleteOp) run(t *test, h *history) {
 	w := t.getWriter(o.writerID)
-	err := w.SingleDelete(o.key, t.writeOpts)
+	var err error
+	if t.testOpts.replaceSingleDelete && o.maybeReplaceDelete {
+		err = w.Delete(o.key, t.writeOpts)
+	} else {
+		err = w.SingleDelete(o.key, t.writeOpts)
+	}
+	// NOTE: even if the SINGLEDEL was replaced with a DELETE, we must still
+	// write the former to the history log. The log line will indicate whether
+	// or not the delete *could* have been replaced. The OPTIONS file should
+	// also be consulted to determine what happened at runtime (i.e. by taking
+	// the logical AND).
 	h.Recordf("%s // %v", o, err)
 }
 
 func (o *singleDeleteOp) String() string {
-	return fmt.Sprintf("%s.SingleDelete(%q)", o.writerID, o.key)
+	return fmt.Sprintf("%s.SingleDelete(%q, %v /* maybeReplaceDelete */)", o.writerID, o.key, o.maybeReplaceDelete)
 }
 
 // deleteRangeOp models a Write.DeleteRange operation.
diff --git a/internal/metamorphic/options.go b/internal/metamorphic/options.go
index 3bec7bb5..868eb371 100644
--- a/internal/metamorphic/options.go
+++ b/internal/metamorphic/options.go
@@ -39,6 +39,9 @@ func parseOptions(opts *testOptions, data string) error {
 			case "TestOptions.ingest_using_apply":
 				opts.ingestUsingApply = true
 				return true
+			case "TestOptions.replace_single_delete":
+				opts.replaceSingleDelete = true
+				return true
 			default:
 				return false
 			}
@@ -50,7 +53,7 @@ func parseOptions(opts *testOptions, data string) error {
 
 func optionsToString(opts *testOptions) string {
 	str := opts.opts.String()
-	if opts.strictFS || opts.ingestUsingApply {
+	if opts.strictFS || opts.ingestUsingApply || opts.replaceSingleDelete {
 		str += "\n[TestOptions]\n"
 	}
 	if opts.strictFS {
@@ -59,6 +62,9 @@ func optionsToString(opts *testOptions) string {
 	if opts.ingestUsingApply {
 		str += "  ingest_using_apply=true\n"
 	}
+	if opts.replaceSingleDelete {
+		str += "  replace_single_delete=true\n"
+	}
 	return str
 }
 
@@ -79,6 +85,8 @@ type testOptions struct {
 	strictFS bool
 	// Use Batch.Apply rather than DB.Ingest.
 	ingestUsingApply bool
+	// Replace a SINGLEDEL with a DELETE.
+	replaceSingleDelete bool
 }
 
 func standardOptions() []*testOptions {
@@ -165,6 +173,10 @@ func standardOptions() []*testOptions {
 		19: `
 [TestOptions]
   ingest_using_apply=true
+`,
+		20: `
+[TestOptions]
+  replace_single_delete=true
 `,
 	}
 
@@ -214,5 +226,6 @@ func randomOptions(rng *rand.Rand) *testOptions {
 		opts.DisableWAL = false
 	}
 	testOpts.ingestUsingApply = rng.Intn(2) != 0
+	testOpts.replaceSingleDelete = rng.Intn(2) != 0
 	return testOpts
 }
diff --git a/internal/metamorphic/parser.go b/internal/metamorphic/parser.go
index 6fd65347..b811810a 100644
--- a/internal/metamorphic/parser.go
+++ b/internal/metamorphic/parser.go
@@ -97,7 +97,7 @@ func opArgs(op op) (receiverID *objID, targetID *objID, args []interface{}) {
 	case *iterSetBoundsOp:
 		return &t.iterID, nil, []interface{}{&t.lower, &t.upper}
 	case *singleDeleteOp:
-		return &t.writerID, nil, []interface{}{&t.key}
+		return &t.writerID, nil, []interface{}{&t.key, &t.maybeReplaceDelete}
 	}
 	panic(fmt.Sprintf("unsupported op type: %T", op))
 }
@@ -262,6 +262,14 @@ func (p *parser) parseArgs(op op, methodName string, args []interface{}) {
 				*t = []byte(s)
 			}
 
+		case *bool:
+			_, lit := p.scanToken(token.IDENT)
+			b, err := strconv.ParseBool(lit)
+			if err != nil {
+				panic(err)
+			}
+			*t = b
+
 		case *objID:
 			pos, lit := p.scanToken(token.IDENT)
 			*t = p.parseObjID(pos, lit)


Test patch:
diff --git a/internal/metamorphic/meta_test.go b/internal/metamorphic/meta_test.go
index 45ba3812..aacf56d5 100644
--- a/internal/metamorphic/meta_test.go
+++ b/internal/metamorphic/meta_test.go
@@ -256,8 +256,10 @@ func TestMeta(t *testing.T) {
 		})
 	}
 
-	// Perform runs with random options.
-	for i := 0; i < 20; i++ {
+	// Perform runs with random options. We make an arbitrary choice to run with
+	// as many random options as we have standard options.
+	nOpts := len(options)
+	for i := 0; i < nOpts; i++ {
 		name := fmt.Sprintf("random-%03d", i)
 		names = append(names, name)
 		opts := randomOptions(rng)