This change is 

All committers have signed the CLA.

(Don't mind me—edited the description to align the benchmark results.)

Looks good overall. I think we want to do this even though the performance benefit over using a normal batch is small. It ensures that read-only operations are truly read-only. Some comments below, the only major one being that Close needs to destroy the iterators.

Review status: 0 of 6 files reviewed at latest revision, 4 unresolved discussions, some commit checks pending.

pkg/storage/replica.go, line 2285 at r1 (raw file):

	var result EvalResult
	rec := ReplicaEvalContext{r, spans}
	br, result, pErr = evaluateBatch(ctx, storagebase.CmdIDKey(""), r.store.Engine().NewReadOnly(), rec, nil, ba)

Per my other comment, we need to Close the read only structure in order to destroy the RocksDB iterators. This will probably look something like:

  readOnly := r.store.Engine().NewReadOnly()
  defer readOnly.Close()

pkg/storage/engine/batch_test.go, line 235 at r1 (raw file):

func TestReadOnlyBasics(t *testing.T) {
	defer leaktest.AfterTest(t)()
	testReadOnlyBasics(t)

There is no need for the indirection here. It is done for some of the other tests where we're passing in different values. Just inline the body of testReadOnlyBasics here.

pkg/storage/engine/engine.go, line 197 at r1 (raw file):

	// them atomically on a call to Commit().
	NewBatch() Batch
	NewReadOnly() ReadWriter

This method needs a documentation comment.

pkg/storage/engine/rocksdb.go, line 649 at r1 (raw file):

}

func (r *rocksDBReadOnly) Close() {}

We do need to destroy the iterators here. Something like:

	if i := &r.prefixIter.rocksDBIterator; i.iter != nil {
		i.destroy()
	}
	if i := &r.normalIter.rocksDBIterator; i.iter != nil {
		i.destroy()
	}

This also means we need to ensure we Close the ReadOnly in Replica.executeReadOnlyBatch.

Comments from Reviewable

Review status: 0 of 6 files reviewed at latest revision, 4 unresolved discussions, some commit checks pending.

pkg/storage/replica.go, line 2285 at r1 (raw file):

Done.

pkg/storage/engine/batch_test.go, line 235 at r1 (raw file):

Done.

pkg/storage/engine/engine.go, line 197 at r1 (raw file):

Done.

pkg/storage/engine/rocksdb.go, line 649 at r1 (raw file):

changed Close to do this as well as set a boolean flag.

Comments from Reviewable

Review status: 0 of 6 files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

pkg/storage/engine/rocksdb.go, line 649 at r1 (raw file):

Done.

Comments from Reviewable

One final bit is to tie the PR to the issue. Add the following to the end of the commit message:

Fixes #6803

Review status: 0 of 6 files reviewed at latest revision, all discussions resolved, some commit checks pending.

Comments from Reviewable

@jonderry There are a few test failures (mostly style issues) that need to be fixed up before this can be merged.

@jonderry I fixed the style issues. I'll merge this when green.