storage: Refactor resolve write intent options

[KaiSun314]

In this PR, we add MVCCResolveWriteIntentOptions to MVCCResolveWriteIntent and MVCCResolveWriteIntentRangeOptions to MVCCResolveWriteIntentRange. Moreover, we additionally return numBytes and resumeSpan in MVCCResolveWriteIntent and numBytes and resumeReason in MVCCResolveWriteIntentRange, but these return values are currently unused and serve as a placeholder in refactoring, but will be used in the future.

Informs: #77228

Release note: None

[blathers-crl]

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

I have added a few people who may be able to assist in reviewing:

• @nvanbenschoten (author of kv: Support byte size pagination with intent resolution #77228)
• @erikgrinaker (commented on kv: Support byte size pagination with intent resolution #77228)

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[cockroach-teamcity]

This change is 

[nvanbenschoten]

👋 thanks!

Reviewed 16 of 16 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker and @KaiSun314)

---

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

	if opts.MaxKeys < 0 {
		resumeSpan := intent.Span // don't inline or `intent` would escape to heap
		return 0, 0, &resumeSpan, roachpb.RESUME_UNKNOWN, nil

nit: It's slightly more idiomatic to return 0 instead of roachpb.RESUME_UNKNOWN in these cases, because the zero value of the enum is usable.

---

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

		if opts.MaxKeys > 0 && numKeys == opts.MaxKeys {
			// We could also compute a tighter nextKey here if we wanted to.
			return numKeys, 0, &roachpb.Span{Key: lastResolvedKey.Next(), EndKey: intentEndKey}, roachpb.RESUME_UNKNOWN, nil

Should this be RESUME_KEY_LIMIT?

[blathers-crl]

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[KaiSun314]

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

---

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

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Should this be RESUME_KEY_LIMIT?

I was thinking in this PR, I would always return 0 for the resumeReason and comment that resumeReason is currently unused (to ensure this PR is purely refactoring). In the next PR that adds byte pagination, I will return the appropriate RESUME_KEY_LIMIT / RESUME_BYTE_LIMIT.

[nvanbenschoten]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @erikgrinaker and @KaiSun314)

---

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

Previously, KaiSun314 (Kai Sun) wrote…

I was thinking in this PR, I would always return 0 for the resumeReason and comment that resumeReason is currently unused (to ensure this PR is purely refactoring). In the next PR that adds byte pagination, I will return the appropriate RESUME_KEY_LIMIT / RESUME_BYTE_LIMIT.

I slightly prefer returning RESUME_KEY_LIMIT here, because that's the resume reason. Each PR should stand on its own, and this one is arguably incorrect if it doesn't report the correct resume reason. Put another way, if we don't want to return a resume reason in this PR, it shouldn't be part of the function signature at all yet.

[sumeerbhola]

I did a quick read -- nothing to add beyond @nvanbenschoten's existing comment.

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

[KaiSun314]

Thank you so much for the review Nathan and Sumeer!

bors r=nvanbenschoten, sumeerbhola

[craig]

🔒 Permission denied

Existing reviewers: click here to make KaiSun314 a reviewer

[erikgrinaker]

bors r+

[craig]

This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried.

Additional information:

[erikgrinaker]

Bors is pretty messed up atm. Let's try again tomorrow.

[erikgrinaker]

Let's try this again.

bors r+ single on

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)