storage/metamorphic: Run pebble with different options

[itsbilal]

This change updates the MVCC metamorphic tests to run Pebble
with different, random options, as well as one configuration
that seems to uncover many issues (the one with low
TargetFileSizes on all levels).

Also fixes batchCommitOp to actually commit the batch
passed in.

Release note: None

Release justification: Safe for this release because it only
updates testing code.

[cockroach-teamcity]

This change is 

[itsbilal]

This uncovered one issue in MVCCReverseScanning where we return incorrect key/value byte slices that happen to be right at the start of an L6 SSTable. It'll probably fail CI since it's pretty reproducible, until I investigate and fix that. But otherwise this is the general idea.

[petermattis]

Looks good. Interested to find out what the reverse scanning problem is.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @itsbilal, @petermattis, and @sumeerbhola)

---

pkg/storage/metamorphic/generator.go, line 91 at r1 (raw file):

	opts.LBaseMaxBytes = rngIntRange(rng, 1 << 8, 128 << 20)
	opts.L0CompactionThreshold = int(rngIntRange(rng, 1, 8))
	opts.L0StopWritesThreshold = int(rngIntRange(rng, 1, 32))

I think there are constraints on the relationships of some of these settings. Take a look at pebble/internal/metamorphic/options.go:randomOptions().

---

pkg/storage/metamorphic/operations.go, line 441 at r1 (raw file):

	}
	batch := b.m.getReadWriter(b.id).(storage.Batch)
	if err := batch.Commit(true); err != nil {

We weren't committing batches before?

---

pkg/storage/metamorphic/operations.go, line 442 at r1 (raw file):

	batch := b.m.getReadWriter(b.id).(storage.Batch)
	if err := batch.Commit(true); err != nil {
		return fmt.Sprintf("error: %s", err.Error())

This can probably just be return err.Error().

[itsbilal]

Tests should be fixed once I rebase this on #46160 . For the record, the reverse scanning problem I alluded to above was cockroachdb/pebble#572 .

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @petermattis and @sumeerbhola)

---

pkg/storage/metamorphic/generator.go, line 91 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

I think there are constraints on the relationships of some of these settings. Take a look at pebble/internal/metamorphic/options.go:randomOptions().

Done. I matched the style there of doing 1 << rngIntRange(x,y) instead of rngIntRange(1 << x, 1 << y), as well as ensured that all of these adhere to the bounds specified there.

---

pkg/storage/metamorphic/operations.go, line 441 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

We weren't committing batches before?

We were, until the big generation/execution refactor where I forgot to copy this over. My bad. Turns out we did uncover some bugs as a result of not committing batches; the ingestion range tombstone elision one would be much harder to detect if we were committing all batches, since it increased the proportion of ingestions to puts. Note that puts right on the engine were always going through.

---

pkg/storage/metamorphic/operations.go, line 442 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

This can probably just be return err.Error().

Done.

[petermattis]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @sumeerbhola)

---

pkg/storage/metamorphic/generator.go, line 91 at r1 (raw file):

Previously, itsbilal (Bilal Akhtar) wrote…

Done. I matched the style there of doing 1 << rngIntRange(x,y) instead of rngIntRange(1 << x, 1 << y), as well as ensured that all of these adhere to the bounds specified there.

Do you need something like:

	if opts.L0StopWritesThreshold < opts.L0CompactionThreshold {
		opts.L0StopWritesThreshold = opts.L0CompactionThreshold
	}

---

pkg/storage/metamorphic/generator.go, line 115 at r2 (raw file):

	defer opts.Cache.Unref()

	fmt.Printf("pebble options:\n%s\n", opts.String())

Did you intend to leave this in the code?

[itsbilal]

TFTR!

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @petermattis and @sumeerbhola)

---

pkg/storage/metamorphic/generator.go, line 91 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Do you need something like:

	if opts.L0StopWritesThreshold < opts.L0CompactionThreshold {
		opts.L0StopWritesThreshold = opts.L0CompactionThreshold
	}

Right, didn't realize that case could happen. Done.

---

pkg/storage/metamorphic/generator.go, line 115 at r2 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Did you intend to leave this in the code?

Since it's also present in the OPTIONS file, not really. Removed.

[itsbilal]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)