storage: restore optimization for inline puts/deletes

[sumeerbhola]

We had an optimization that avoided constructing an iterator over the locktable key space when doing inline puts/deletes. This was lost in ac14ebb that constructed a lockTableKeyScanner even for inline puts/deletes. The optimization is restored here to fix the regression discussed in #111919 (comment).

Informs #111919

Epic: none

Release note: None

[cockroach-teamcity]

This change is 

[sumeerbhola]

test failure is unrelated #111481

[jbowens]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @nvanbenschoten)

[sumeerbhola]

TFTR!

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

[sumeerbhola]

bors r=jbowens

[craig]

Build failed:

• Bazel Essential CI (Cockroach)

[sumeerbhola]

Failure is unrelated #112659

[nvanbenschoten]

Thanks for tracking this down!

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @sumeerbhola)

---

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

) (int64, error) {
	if timestamp.IsEmpty() {
		return 0, errors.Errorf("increment not supported for inline values")

Has this always been a restriction or are you introducing it now? What is preventing inline increments from working correctly?

[sumeerbhola]

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

---

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

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Has this always been a restriction or are you introducing it now? What is preventing inline increments from working correctly?

I introduced it. I was assuming that all inline data was stored as MVCCMetadata so wouldn't support increment anyway.
Should I remove this restriction?

[nvanbenschoten]

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

---

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

I was assuming that all inline data was stored as MVCCMetadata

This is true, but why would that preclude an increment operation over the value in MVCCMetadata.RawBytes?

[sumeerbhola]

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

---

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

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

I was assuming that all inline data was stored as MVCCMetadata

This is true, but why would that preclude an increment operation over the value in MVCCMetadata.RawBytes?

Ah, I missed that we are doing makeOptionalValue(roachpb.Value{RawBytes: meta.RawBytes}). Removed the restriction.

[nvanbenschoten]

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

---

pkg/storage/testdata/mvcc_histories/replicated_locks line 455 at r2 (raw file):

meta: "k4"/0,0 -> txn={id=00000001 key=/Min iso=Serializable pri=0.00000000 epo=1 ts=10.000000000,0 min=0,0 seq=2} ts=10.000000000,0 del=false klen=12 vlen=13 ih={{1 /BYTES/v4}} mergeTs=<nil> txnDidNotUpdateMeta=false
data: "k4"/10.000000000,0 -> /BYTES/v4_prime
error: (*withstack.withStack:) "k1"/0,0: put is inline=true, but existing value is inline=false

This error is unrelated to replicated locks, so it would be better if we tested this case and its inverse in mvcc_histories/inline, but I won't block the PR on that.

[sumeerbhola]

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

---

pkg/storage/testdata/mvcc_histories/replicated_locks line 455 at r2 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

This error is unrelated to replicated locks, so it would be better if we tested this case and its inverse in mvcc_histories/inline, but I won't block the PR on that.

Done

[sumeerbhola]

TFTRs!

[sumeerbhola]

bors r=jbowens,nvanbenschoten

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)

[blathers-crl]

Encountered an error creating backports. Some common things that can go wrong:

1. The backport branch might have already existed.
2. There was a merge conflict.
3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

---

error creating merge commit from 3b6e770 to blathers/backport-release-23.2-112586: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.2.x failed. See errors above.

---

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