metamorphic: add IngestAndExcise, make EFOS determinitic

This change updates the metamorphic tests to support a new
useExcise testOption and a new IngestAndExcise operation, which
calls db.IngestAndExcise() if useExcise is true, or calls it
with an sstable containing DeleteRange/RangeKeyDelete for the
same span if useExcise is false.

Furthermore, a new private option is added, efosAlwaysCreatesIterators.
If the test option for useExcise (or the previously added useSharedReplicate)
is enabled, one of these two things happen to guarantee EFOS determinism
around excises:

1) If efosAlwaysCreatesIterators is true, the behaviour of EFOS
is changed to always hold onto a readState and never return
errors from NewIter.
2) If efosAlwaysCreatesIterators is false, regular EFOS behaviour
is retained, but a flush is completed after every EFOS instantiation
to guarantee that it becomes excise-immune right at instantiation.

Note that regular snapshots are never created when useExcise
or useSharedReplicate is true.

Fixes #2885.

db.go

@@ -1001,9 +1001,12 @@ var iterAllocPool = sync.Pool{
 //     and the specified seqNum will be used as the snapshot seqNum.
 //   - EFOS in file-only state: Only `seqNum` and `vers` are set. All the
 //     relevant SSTs are referenced by the *version.
+//   - EFOS that has been excised but is in alwaysCreateIters mode (tests only).
+//     Only `seqNum` and `readState` are set.
 type snapshotIterOpts struct {
-	seqNum uint64
-	vers   *version
+	seqNum    uint64
+	vers      *version
+	readState *readState
 }
 
 type batchIterOpts struct {
@@ -1057,8 +1060,13 @@ func (d *DB) newIter(
 		// files in the associated version from being deleted if there is a current
 		// compaction. The readState is unref'd by Iterator.Close().
 		if internalOpts.snapshot.vers == nil {
-			// NB: loadReadState() calls readState.ref().
-			readState = d.loadReadState()
+			if internalOpts.snapshot.readState != nil {
+				readState = internalOpts.snapshot.readState
+				readState.ref()
+			} else {
+				// NB: loadReadState() calls readState.ref().
+				readState = d.loadReadState()
+			}
 		} else {
 			// vers != nil
 			internalOpts.snapshot.vers.Ref()
@@ -1289,7 +1297,12 @@ func (d *DB) newInternalIter(
 	// compaction. The readState is unref'd by Iterator.Close().
 	var readState *readState
 	if sOpts.vers == nil {
-		readState = d.loadReadState()
+		if sOpts.readState != nil {
+			readState = sOpts.readState
+			readState.ref()
+		} else {
+			readState = d.loadReadState()
+		}
 	}
 	if sOpts.vers != nil {
 		sOpts.vers.Ref()

internal/metamorphic/meta_test.go

@@ -76,13 +76,15 @@ func TestMetaTwoInstance(t *testing.T) {
 	case runOnceFlags.Compare != "":
 		runDirs := strings.Split(runOnceFlags.Compare, ",")
 		onceOpts := runOnceFlags.MakeRunOnceOptions()
+		onceOpts = append(onceOpts, metamorphic.MultiInstance(2))
 		metamorphic.Compare(t, runOnceFlags.Dir, runOnceFlags.Seed, runDirs, onceOpts...)
 
 	case runOnceFlags.RunDir != "":
 		// The --run-dir flag is specified either in the child process (see
 		// runOptions() below) or the user specified it manually in order to re-run
 		// a test.
 		onceOpts := runOnceFlags.MakeRunOnceOptions()
+		onceOpts = append(onceOpts, metamorphic.MultiInstance(2))
 		metamorphic.RunOnce(t, runOnceFlags.RunDir, runOnceFlags.Seed, filepath.Join(runOnceFlags.RunDir, "history"), onceOpts...)
 
 	default:

metamorphic/config.go

@@ -45,6 +45,7 @@ const (
 	writerDelete
 	writerDeleteRange
 	writerIngest
+	writerIngestAndExcise
 	writerMerge
 	writerRangeKeyDelete
 	writerRangeKeySet
@@ -154,6 +155,7 @@ func defaultConfig() config {
 			writerDelete:                100,
 			writerDeleteRange:           50,
 			writerIngest:                100,
+			writerIngestAndExcise:       50,
 			writerMerge:                 100,
 			writerRangeKeySet:           10,
 			writerRangeKeyUnset:         10,

metamorphic/generator.go

@@ -171,6 +171,7 @@ func generate(rng *rand.Rand, count uint64, cfg config, km *keyManager) []op {
 		writerDelete:                g.writerDelete,
 		writerDeleteRange:           g.writerDeleteRange,
 		writerIngest:                g.writerIngest,
+		writerIngestAndExcise:       g.writerIngestAndExcise,
 		writerMerge:                 g.writerMerge,
 		writerRangeKeyDelete:        g.writerRangeKeyDelete,
 		writerRangeKeySet:           g.writerRangeKeySet,
@@ -1174,7 +1175,7 @@ func (g *generator) replicate() {
 	var startKey, endKey []byte
 	startKey = g.randKeyToRead(0.001) // 0.1% new keys
 	endKey = g.randKeyToRead(0.001)   // 0.1% new keys
-	for g.cmp(startKey, endKey) == 0 {
+	for g.equal(startKey, endKey) {
 		endKey = g.randKeyToRead(0.01) // 1% new keys
 	}
 	if g.cmp(startKey, endKey) > 0 {
@@ -1228,28 +1229,15 @@ func (g *generator) newSnapshot() {
 		snapID: snapID,
 	}
 
-	// With 75% probability, impose bounds on the keys that may be read with the
-	// snapshot. Setting bounds allows some runs of the metamorphic test to use
-	// a EventuallyFileOnlySnapshot instead of a Snapshot, testing equivalence
-	// between the two for reads within those bounds.
-	//
-	// If we're in multi-instance mode, we must always create bounds, as we will
-	// always create EventuallyFileOnlySnapshots to allow commands that use excises
-	// (eg. replicateOp) to work.
-	if g.rng.Float64() < 0.75 || g.dbs.Len() > 1 {
-		s.bounds = g.generateDisjointKeyRanges(
-			g.rng.Intn(5) + 1, /* between 1-5 */
-		)
-		g.snapshotBounds[snapID] = s.bounds
-	}
+	// Impose bounds on the keys that may be read with the snapshot. Setting bounds
+	// allows some runs of the metamorphic test to use a EventuallyFileOnlySnapshot
+	// instead of a Snapshot, testing equivalence between the two for reads within
+	// those bounds.
+	s.bounds = g.generateDisjointKeyRanges(
+		g.rng.Intn(5) + 1, /* between 1-5 */
+	)
+	g.snapshotBounds[snapID] = s.bounds
 	g.add(s)
-	if g.dbs.Len() > 1 {
-		// Do a flush after each EFOS, if we're in multi-instance mode. This limits
-		// the testing area of EFOS, but allows them to be used alongside operations
-		// that do an excise (eg. replicateOp). This will be revisited when
-		// https://github.com/cockroachdb/pebble/issues/2885 is implemented.
-		g.add(&flushOp{dbID})
-	}
 }
 
 func (g *generator) snapshotClose() {
@@ -1454,6 +1442,34 @@ func (g *generator) writerIngest() {
 	})
 }
 
+func (g *generator) writerIngestAndExcise() {
+	if len(g.liveBatches) == 0 {
+		return
+	}
+
+	dbID := g.dbs.rand(g.rng)
+	batchID := g.liveBatches.rand(g.rng)
+	g.removeBatchFromGenerator(batchID)
+
+	start := g.randKeyToWrite(0.001)
+	end := g.randKeyToWrite(0.001)
+	for g.equal(start, end) {
+		end = g.randKeyToWrite(0.001)
+	}
+	if g.cmp(start, end) > 0 {
+		start, end = end, start
+	}
+	derivedDBID := g.objDB[batchID]
+
+	g.add(&ingestAndExciseOp{
+		dbID:        dbID,
+		batchID:     batchID,
+		derivedDBID: derivedDBID,
+		exciseStart: start,
+		exciseEnd:   end,
+	})
+}
+
 func (g *generator) writerMerge() {
 	if len(g.liveWriters) == 0 {
 		return

metamorphic/key_manager.go

@@ -406,6 +406,10 @@ func (k *keyManager) update(o op) {
 			k.mergeKeysInto(batchID, s.dbID)
 		}
 		k.checkForDelOrSingleDelTransitionInDB(s.dbID)
+	case *ingestAndExciseOp:
+		// Merge all keys in the batch with the keys in the DB.
+		k.mergeKeysInto(s.batchID, s.dbID)
+		k.checkForDelOrSingleDelTransitionInDB(s.dbID)
 	case *applyOp:
 		// Merge the keys from this writer into the parent writer.
 		k.mergeKeysInto(s.batchID, s.writerID)
@@ -517,6 +521,8 @@ func opWrittenKeys(untypedOp op) [][]byte {
 	case *flushOp:
 	case *getOp:
 	case *ingestOp:
+	case *ingestAndExciseOp:
+		return [][]byte{t.exciseStart, t.exciseEnd}
 	case *initOp:
 	case *iterFirstOp:
 	case *iterLastOp: