kv: bump ReplicaChecksumVersion, disable cross v23.2/v24.1 consistency checks

[nvanbenschoten]

Informs #101938.
Fixes #117302.

This commit bumps the ReplicaChecksumVersion to disable replica consistency checks between v23.2 and v24.1 nodes when in a mixed-version cluster. This avoids the backwards incompatibility discussed in #117302.

While here and permitted to change the replica consistency check logic, we unset the Synthetic flag from RangeAppliedState.RaftClosedTimestamp during stats-only consistency checks. This form of consistency check is rarely used, but this prevents it from causing trouble with #101938.

While here and allowed to change the consistency check hash computation, we also switch from using the LegacyTimestamp encoding to the Timestamp encoding for the hash contribution of MVCCKeys.

Release note: None

[blathers-crl]

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

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

[cockroach-teamcity]

This change is 

[nvanbenschoten]

@erikgrinaker @pav-kv I'd like to get your input on this option for resolving #117302. The benefit is that it's simple and it doesn't require a revert of #105523 ¹. The downside is that it uses the ReplicaChecksumVersion mechanism to disallow cross v23.1/v23.2 consistency checks, so we'll lose some consistency check coverage in mixed-version clusters that are running multiple binary versions. I'm leaning towards this being the right approach because clusters aren't meant to hang around in this state for a long time (just during the rolling restart), so the simplicity argument would win out. Curious to get your thoughts.

In the meantime, I'll run this with #117294 for a few dozen iterations overnight to confirm that it resolves the inconsistency.

Footnotes

1. this would push back https://github.com/cockroachdb/cockroach/issues/101938 by a major releases and make the eventual migration more involved. If we never wanted to bump ReplicaChecksumVersion then we'd need to add a targeted version gate to consistency check requests to determine whether v24.1 nodes should ignore the synthetic bit in mvcc keys or not. Only in v24.2 could we then remove the field from the proto. ↩

[erikgrinaker]

This seems fine to me. It's why ReplicaChecksumVersion exists in the first place.

You've verified that we don't have any further dependencies on Synthetic here?

[pav-kv]

SGTM

[pav-kv]

Since we're bumping the version anyway, do we want to do any drive-by changes/refactorings to the hash computation? For example, #117302 mentions we're using ToLegacyTimestamp. Can we untangle from something named "legacy"?

[nvanbenschoten]

@erikgrinaker and @pav-kv, I've updated this PR with your suggestions. Now that #117324 has landed and all of the synthetic timestamp migration will land in v24.1, #117302 is no longer an issue for v23.1/v23.2 mixed-version clusters, but it will still be an issue for the eventual v23.2/v24.1 state. This PR will avoid that, so I'm planning to land it just on master.

PTAL when you get a chance.

[erikgrinaker]

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

[nvanbenschoten]

TFTR!

bors r+

[craig]

This PR was included in a batch that timed out, it will be automatically retried

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)