Merge #102155

102155: kv: delete `(hlc.Timestamp).DeprecatedTryToClockTimestamp` r=nvanbenschoten a=nvanbenschoten Informs #101938. This function has been possible to delete since v23.1. Release note: None Co-authored-by: Nathan VanBenschoten <[email protected]>
cockroachdb · Apr 26, 2023 · 2f42310 · 2f42310
2 parents 2066afb + b5ae9a9
commit 2f42310
Show file tree

Hide file tree

Showing 3 changed files with 3 additions and 34 deletions.
diff --git a/pkg/clusterversion/cockroach_versions.go b/pkg/clusterversion/cockroach_versions.go
@@ -184,8 +184,6 @@ const (
 	// TODODelete_V22_2Start demarcates work towards CockroachDB v22.2.
 	TODODelete_V22_2Start
 
-	// TODODelete_V22_2LocalTimestamps enables the use of local timestamps in MVCC values.
-	TODODelete_V22_2LocalTimestamps
 	// TODODelete_V22_2PebbleFormatSplitUserKeysMarkedCompacted updates the Pebble format
 	// version that recombines all user keys that may be split across multiple
 	// files into a single table.
@@ -613,10 +611,6 @@ var rawVersionsSingleton = keyedVersions{
 		Key:     TODODelete_V22_2Start,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 2},
 	},
-	{
-		Key:     TODODelete_V22_2LocalTimestamps,
-		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 4},
-	},
 	{
 		Key:     TODODelete_V22_2PebbleFormatSplitUserKeysMarkedCompacted,
 		Version: roachpb.Version{Major: 22, Minor: 1, Internal: 6},

diff --git a/pkg/kv/kvserver/store_send.go b/pkg/kv/kvserver/store_send.go
@@ -14,7 +14,6 @@ import (
 	"context"
 	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvadmission"
@@ -91,25 +90,13 @@ func (s *Store) SendWithWriteBytes(
 	// Update our clock with the incoming request timestamp. This advances the
 	// local node's clock to a high water mark from all nodes with which it has
 	// interacted.
-	baClockTS := ba.Now
-	if baClockTS.IsEmpty() && !s.ClusterSettings().Version.IsActive(ctx, clusterversion.TODODelete_V22_2LocalTimestamps) {
-		// TODO(nvanbenschoten): remove this in v23.1. v21.2 nodes will still send
-		// requests without a Now field. This is not necessary for correctness now
-		// that local timestamps pulled from the leaseholder's own HLC are used in
-		// conjunction with observed timestamps to prevent stale reads, but using
-		// this timestamp when available can help stabilize HLCs.
-		//
-		// NOTE: we version gate this so that no test hits this branch and relies on
-		// this behavior.
-		baClockTS, _ = ba.Timestamp.DeprecatedTryToClockTimestamp()
-	}
-	if !baClockTS.IsEmpty() {
+	if !ba.Now.IsEmpty() {
 		if s.cfg.TestingKnobs.DisableMaxOffsetCheck {
-			s.cfg.Clock.Update(baClockTS)
+			s.cfg.Clock.Update(ba.Now)
 		} else {
 			// If the command appears to come from a node with a bad clock,
 			// reject it instead of updating the local clock and proceeding.
-			if err := s.cfg.Clock.UpdateAndCheckMaxOffset(ctx, baClockTS); err != nil {
+			if err := s.cfg.Clock.UpdateAndCheckMaxOffset(ctx, ba.Now); err != nil {
 				return nil, nil, kvpb.NewError(err)
 			}
 		}

diff --git a/pkg/util/hlc/timestamp.go b/pkg/util/hlc/timestamp.go
@@ -397,18 +397,6 @@ func (LegacyTimestamp) SafeValue() {}
 // Synthetic flag set to false.
 type ClockTimestamp Timestamp
 
-// DeprecatedTryToClockTimestamp attempts to downcast a Timestamp into a
-// ClockTimestamp. Returns the result and a boolean indicating whether the cast
-// succeeded.
-// TODO(nvanbenschoten): remove this in v23.1 when we remove the synthetic
-// timestamp bit.
-func (t Timestamp) DeprecatedTryToClockTimestamp() (ClockTimestamp, bool) {
-	if t.Synthetic {
-		return ClockTimestamp{}, false
-	}
-	return ClockTimestamp(t), true
-}
-
 // UnsafeToClockTimestamp converts a Timestamp to a ClockTimestamp, regardless
 // of whether such a cast would be legal according to the Synthetic flag. The
 // method should only be used in tests.