kv: don't pass clock information through Raft log

[nvanbenschoten]

This commit eliminates the primary mechanism that we use to pass clock information from a leaseholder, through Raft log entries, to a Range's followers. As we found in #72278, this was only needed for correctness in a few specific cases — namely lease transfers and range merges. These two operations continue to pass clock signals through more explicit channels, but we remove the unnecessary general case.

The allows us to remote one of the two remaining places where we convert a Timestamp to a ClockTimestamp through the TryToClockTimestamp method. As outlined in #72121 (comment), I would like to remove ability to downcast a "data-plane" Timestamp to a "control-plane" CloudTimestamp entirely. This will clarify the role of ClockTimestamps in the system and clean up the channels through which clock information is passed between nodes.

The other place where we cast from Timestamp to ClockTimesatmp is in Store.Send, at the boundary of KV RPCs. I would also like to get rid of this, but doing so needs to wait on #73732.

[cockroach-teamcity]

This change is 

[tbg]

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

---

pkg/kv/kvserver/replica_proposal.go, line 864 at r2 (raw file):

			res.Replicated.WriteTimestamp = ba.WriteTimestamp()
		} else {
			if !r.ClusterSettings().Version.IsActive(ctx, clusterversion.DontProposeWriteTimestampForLeaseTransfers) {

IsActive is moderately expensive (has to unmarshal a protobuf). I assume this code path is "cold enough" for this not to matter (since most requests will apply the timestamp cache) but wanted to point it out anyway.

---

pkg/kv/kvserver/kvserverpb/proposer_kv.proto, line 132 at r2 (raw file):

  // of the command against the GC threshold and to update the followers'
  // clocks. Only set if the request that produced this command is a write that
  // cares about the timestamp cache.

This is deprecated now, right?

[nvanbenschoten]

TFTR!

bors r+

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

---

pkg/kv/kvserver/replica_proposal.go, line 864 at r2 (raw file):

Previously, tbg (Tobias Grieger) wrote…

IsActive is moderately expensive (has to unmarshal a protobuf). I assume this code path is "cold enough" for this not to matter (since most requests will apply the timestamp cache) but wanted to point it out anyway.

Yeah, this is cold enough that unmarshalling a proto seems ok. Has IsActive gotten more expensive recently though? I wasn't aware.

---

pkg/kv/kvserver/kvserverpb/proposer_kv.proto, line 132 at r2 (raw file):

Previously, tbg (Tobias Grieger) wrote…

This is deprecated now, right?

Not fully. It's still used for real MVCC writes that use it to check against the GC threshold (below Raft in checkForcedErr), to assert against closed timestamp violations (assertNoWriteBelowClosedTimestamp), and now for AddSSTable requests with the WriteAtRequestTimestamp flag set.

This first use seemed dubious to me and Andrei, but our attempts to understand it enough to get rid of it were unsuccessful, so I decided not to pick that fight in this PR. The HLC is a formidable enough foe 1-on-1.

[craig]

Build succeeded:

• GitHub CI (Cockroach)