Nice. - just some documentation nits.

Reviewed 11 of 11 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @jbowens)

-- commits line 5 at r1:

Batch sequence numbers may force this method to allocate under some circumstances

I didn't quite follow where / how this allocation was happening in the batch case. Can you elaborate for my own benefit?

internal/keyspan/span.go line 215 at r1 (raw file):

	} else {
		// Otherwise we check the last key. Since keys are ordered decreasing in
		// sequence number, the last key has the lowest sequence number. If any

I read the last sentence a couple times before realizing that what I was kind of expecting was the contrapositive tacked on the end. Something like: "Or put differently: if the last key is not visible, then no key is visible".

I'm just proving this to myself, so feel free to ignore if you think it's clear.

internal/keyspan/span.go line 268 at r1 (raw file):

}

// CoversAt returns true if the span contains a key that is visible at the

nit: Possible to add a clarifying parenthetical for "visibility" along the lines of: "(i.e. the key's sequence number is strictly less than the snapshot sequence number)"?

.. or maybe I just need to burn that concept into my brain (> for snapshot visibility rather than >=).

internal/keyspan/span.go line 274 at r1 (raw file):

	for i := range s.Keys {
		if kseq := s.Keys[i].SeqNum(); kseq&base.InternalKeySeqNumBatch != 0 {
			// Only visible batch keys are included when an Iterator's batch spans

What do you think about mentioning the behavior for batches the docstring too? Same for VisibleAt.

internal/keyspan/testdata/visible_at line 21 at r1 (raw file):

1 : false

define

nit: Reading this file in isolation, I'm unlikely to recognize the relevance of the seemingly arbitrary sequence numbers. Worth an additional assertion / "proof" in here that the seqnums are from a batch, and therefore snapshot sequnum is irrelevant?

TFTR!

Reviewable status: 9 of 11 files reviewed, 3 unresolved discussions (waiting on @nicktrav)

-- commits line 5 at r1:

Here's where that allocation happens:

There's more in depth discussion about why this allocation is necessary higher up in Visible:

internal/keyspan/span.go line 268 at r1 (raw file):

I'll add a comment to the body of the function. I think ideally clients don't need to worry about the numerical relationship (eg, > vs >=), and rely on utilities like this that abstract the concept of visibility.

internal/keyspan/span.go line 274 at r1 (raw file):

done

internal/keyspan/testdata/visible_at line 21 at r1 (raw file):

added a comment documenting these as batch sequence numbers