kvserver,roachpb,storage: add IsSpanEmptyRequest to check for any data

[ajwerner]

This commit adds a new request, IsSpanEmptyRequest, which checks to see if
there is any data in a key span of any kind. It ignore the GC threshold, and
operates across all versions.

Fixes #85726

Release note: None

[cockroach-teamcity]

This change is 

[erikgrinaker]

Overall looks good, thanks!

Reviewed 12 of 12 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)

---

pkg/kv/kvserver/batcheval/cmd_is_span_empty_test.go line 56 at r1 (raw file):

	checkIsEmpty := func(t *testing.T, exp bool, from, to roachpb.Key) {
		var ba kv.Batch
		ba.Header.MaxSpanRequestKeys = 1

This is the only reason the DistSender isn't parallelizing the request. So if we don't set this, we'll get back the number of non-empty ranges, scanned in parallel. I suppose that might make sense, in a way, and we could extend it to also count the number of keys (I think David mentioned wanting this for some purpose). We should mention this in the request comment.

---

pkg/roachpb/api.proto line 203 at r1 (raw file):

}

// IsSpanEmptyResponse is the response to an IsSpanEmptyRequest.

Let's mention that NumKeys is what callers want.

---

pkg/storage/mvcc.go line 5285 at r1 (raw file):

	// If we're not exporting all revisions then we can mask point keys below any
	// MVCC range tombstones, since we don't care about them.
	var rangeKeyMasking hlc.Timestamp

We don't really need this.

---

pkg/storage/mvcc.go line 5294 at r1 (raw file):

		EndTime:              opts.EndTS,
		RangeKeyMaskingBelow: rangeKeyMasking,
		IntentPolicy:         MVCCIncrementalIterIntentPolicyEmit,

Let's surface inline values too, rather than error on them.

---

pkg/storage/testdata/mvcc_histories/clear_range line 41 at r1 (raw file):

run ok
is_span_empty k=a end=+a

We could have some more test cases here (e.g. range keys, time spans etc), but no need to spend time on that now. The implementation is trivial, and MVCCIncrementalIterator itself has plenty of coverage.

[ajwerner]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @erikgrinaker)

---

pkg/kv/kvserver/batcheval/cmd_is_span_empty_test.go line 56 at r1 (raw file):

Previously, erikgrinaker (Erik Grinaker) wrote…

This is the only reason the DistSender isn't parallelizing the request. So if we don't set this, we'll get back the number of non-empty ranges, scanned in parallel. I suppose that might make sense, in a way, and we could extend it to also count the number of keys (I think David mentioned wanting this for some purpose). We should mention this in the request comment.

Yeah, correct. One thing I did is changed the combine method to set the NumKeys back to 1.

---

pkg/roachpb/api.proto line 203 at r1 (raw file):

Previously, erikgrinaker (Erik Grinaker) wrote…

Let's mention that NumKeys is what callers want.

Done.

---

pkg/storage/mvcc.go line 5281 at r1 (raw file):

	var span *tracing.Span
	ctx, span = tracing.ChildSpan(ctx, "MVCCIsSpanEmpty")
	defer span.Finish()

I removed this tracing span too; don't think we need it

Code quote:

	ctx, span = tracing.ChildSpan(ctx, "MVCCIsSpanEmpty")
	defer span.Finish()

---

pkg/storage/mvcc.go line 5285 at r1 (raw file):

Previously, erikgrinaker (Erik Grinaker) wrote…

We don't really need this.

copy-pasta, removed

---

pkg/storage/mvcc.go line 5294 at r1 (raw file):

Previously, erikgrinaker (Erik Grinaker) wrote…

Let's surface inline values too, rather than error on them.

Isn't that what this is doing? What is this comment asking for?

[erikgrinaker]

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)

---

pkg/storage/mvcc.go line 5294 at r1 (raw file):

Previously, ajwerner wrote…

Isn't that what this is doing? What is this comment asking for?

Oh, sorry. We used to have InlinePolicy too, but we now unconditionally error on inline values -- the TBI will skip over inline values most of the time anyway, so this really shouldn't be used with spans that can have inline values at all. May want to call that out on the request.

[ajwerner]

@erikgrinaker anything else you want on this?

[erikgrinaker]

Nah, this looks good. I assume we don't need a version gate because it'll be gated by the broader schema GC version gate?

Reviewed 1 of 1 files at r3, 4 of 4 files at r4, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)

[ajwerner]

TFTR!

bors r=erikgrinaker

Nah, this looks good. I assume we don't need a version gate because it'll be gated by the broader schema GC version gate?

Correct.

[craig]

Build failed (retrying...):

• Bazel Essential CI (Cockroach)

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)