kv: Use strict types for common fields

This PR introduces 3 new typed fields in mvcc.go:
RaftTerm, RaftIndex and LeaseSequence. These fields were previously just
unit64 throughout the code and this made the code harder to read and
risked incorrect conversions.

Epic: none

Release note: None

pkg/cli/debug_check_store.go

@@ -80,11 +80,11 @@ func runDebugCheckStoreCmd(cmd *cobra.Command, args []string) error {
 }
 
 type replicaCheckInfo struct {
-	truncatedIndex uint64
-	appliedIndex   uint64
-	firstIndex     uint64
-	lastIndex      uint64
-	committedIndex uint64
+	truncatedIndex enginepb.RaftIndex
+	appliedIndex   enginepb.RaftIndex
+	firstIndex     enginepb.RaftIndex
+	lastIndex      enginepb.RaftIndex
+	committedIndex enginepb.RaftIndex
 }
 
 type checkInput struct {
@@ -251,7 +251,7 @@ func checkStoreRaftState(
 				if err := kv.Value.GetProto(&hs); err != nil {
 					return err
 				}
-				getReplicaInfo(rangeID).committedIndex = hs.Commit
+				getReplicaInfo(rangeID).committedIndex = enginepb.RaftIndex(hs.Commit)
 			case bytes.Equal(suffix, keys.LocalRaftTruncatedStateSuffix):
 				var trunc roachpb.RaftTruncatedState
 				if err := kv.Value.GetProto(&trunc); err != nil {
@@ -265,7 +265,8 @@ func checkStoreRaftState(
 				}
 				getReplicaInfo(rangeID).appliedIndex = state.RaftAppliedIndex
 			case bytes.Equal(suffix, keys.LocalRaftLogSuffix):
-				_, index, err := encoding.DecodeUint64Ascending(detail)
+				_, uIndex, err := encoding.DecodeUint64Ascending(detail)
+				index := enginepb.RaftIndex(uIndex)
 				if err != nil {
 					return err
 				}

pkg/keys/BUILD.bazel

@@ -17,6 +17,7 @@ go_library(
     deps = [
         "//pkg/kv/kvpb",
         "//pkg/roachpb",
+        "//pkg/storage/enginepb",
         "//pkg/util/encoding",
         "//pkg/util/uuid",
         "@com_github_cockroachdb_errors//:errors",
@@ -37,6 +38,7 @@ go_test(
     deps = [
         "//pkg/kv/kvpb",
         "//pkg/roachpb",
+        "//pkg/storage/enginepb",
         "//pkg/testutils",
         "//pkg/util/bitarray",
         "//pkg/util/duration",

pkg/keys/keys.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/encoding"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
 	"github.com/cockroachdb/errors"
@@ -356,21 +357,21 @@ func RaftLogPrefix(rangeID roachpb.RangeID) roachpb.Key {
 }
 
 // RaftLogKey returns a system-local key for a Raft log entry.
-func RaftLogKey(rangeID roachpb.RangeID, logIndex uint64) roachpb.Key {
+func RaftLogKey(rangeID roachpb.RangeID, logIndex enginepb.RaftIndex) roachpb.Key {
 	return MakeRangeIDPrefixBuf(rangeID).RaftLogKey(logIndex)
 }
 
 // RaftLogKeyFromPrefix returns a system-local key for a Raft log entry, using
 // the provided Raft log prefix.
-func RaftLogKeyFromPrefix(raftLogPrefix []byte, logIndex uint64) roachpb.Key {
-	return encoding.EncodeUint64Ascending(raftLogPrefix, logIndex)
+func RaftLogKeyFromPrefix(raftLogPrefix []byte, logIndex enginepb.RaftIndex) roachpb.Key {
+	return encoding.EncodeUint64Ascending(raftLogPrefix, uint64(logIndex))
 }
 
 // DecodeRaftLogKeyFromSuffix parses the suffix of a system-local key for a Raft
 // log entry and returns the entry's log index.
-func DecodeRaftLogKeyFromSuffix(raftLogSuffix []byte) (uint64, error) {
+func DecodeRaftLogKeyFromSuffix(raftLogSuffix []byte) (enginepb.RaftIndex, error) {
 	_, logIndex, err := encoding.DecodeUint64Ascending(raftLogSuffix)
-	return logIndex, err
+	return enginepb.RaftIndex(logIndex), err
 }
 
 // RaftReplicaIDKey returns a system-local key for a RaftReplicaID.
@@ -1092,7 +1093,7 @@ func (b RangeIDPrefixBuf) RaftLogPrefix() roachpb.Key {
 }
 
 // RaftLogKey returns a system-local key for a Raft log entry.
-func (b RangeIDPrefixBuf) RaftLogKey(logIndex uint64) roachpb.Key {
+func (b RangeIDPrefixBuf) RaftLogKey(logIndex enginepb.RaftIndex) roachpb.Key {
 	return RaftLogKeyFromPrefix(b.RaftLogPrefix(), logIndex)
 }
 

pkg/keys/printer.go

@@ -18,6 +18,7 @@ import (
 	"unicode/utf8"
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/encoding"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
 	"github.com/cockroachdb/errors"
@@ -305,7 +306,7 @@ func raftLogKeyParse(rangeID roachpb.RangeID, input string) (string, roachpb.Key
 	if err != nil {
 		panic(err)
 	}
-	return "", RaftLogKey(rangeID, index)
+	return "", RaftLogKey(rangeID, enginepb.RaftIndex(index))
 }
 
 func raftLogKeyPrint(buf *redact.StringBuilder, key roachpb.Key) {

pkg/keys/printer_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/cockroachdb/apd/v3"
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/bitarray"
 	"github.com/cockroachdb/cockroach/pkg/util/duration"
 	"github.com/cockroachdb/cockroach/pkg/util/encoding"
@@ -250,7 +251,7 @@ func TestPrettyPrint(t *testing.T) {
 
 		{keys.RaftHardStateKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/u/RaftHardState", revertSupportUnknown},
 		{keys.RangeTombstoneKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/u/RangeTombstone", revertSupportUnknown},
-		{keys.RaftLogKey(roachpb.RangeID(1000001), uint64(200001)), "/Local/RangeID/1000001/u/RaftLog/logIndex:200001", revertSupportUnknown},
+		{keys.RaftLogKey(roachpb.RangeID(1000001), enginepb.RaftIndex(200001)), "/Local/RangeID/1000001/u/RaftLog/logIndex:200001", revertSupportUnknown},
 		{keys.RangeLastReplicaGCTimestampKey(roachpb.RangeID(1000001)), "/Local/RangeID/1000001/u/RangeLastReplicaGCTimestamp", revertSupportUnknown},
 
 		{keys.MakeRangeKeyPrefix(roachpb.RKey(tenSysCodec.TablePrefix(42))), `/Local/Range/Table/42`, revertSupportUnknown},

pkg/kv/kvclient/kvcoord/dist_sender_test.go

@@ -39,6 +39,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/rpc/nodedialer"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/sql/sqlliveness"
+	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/util"
@@ -813,7 +814,7 @@ func TestBackoffOnNotLeaseHolderErrorDuringTransfer(t *testing.T) {
 			t.Fatal(err)
 		}
 	}
-	var sequences []roachpb.LeaseSequence
+	var sequences []enginepb.LeaseSequence
 	var testFn simpleSendFn = func(_ context.Context, args *kvpb.BatchRequest) (*kvpb.BatchResponse, error) {
 		reply := &kvpb.BatchResponse{}
 		if len(sequences) > 0 {
@@ -856,12 +857,12 @@ func TestBackoffOnNotLeaseHolderErrorDuringTransfer(t *testing.T) {
 		Settings: cluster.MakeTestingClusterSettings(),
 	}
 	for i, c := range []struct {
-		leaseSequences []roachpb.LeaseSequence
+		leaseSequences []enginepb.LeaseSequence
 		expected       int64
 	}{
-		{[]roachpb.LeaseSequence{2, 1, 2, 3}, 2},
-		{[]roachpb.LeaseSequence{0}, 0},
-		{[]roachpb.LeaseSequence{2, 1, 2, 3, 2}, 3},
+		{[]enginepb.LeaseSequence{2, 1, 2, 3}, 2},
+		{[]enginepb.LeaseSequence{0}, 0},
+		{[]enginepb.LeaseSequence{2, 1, 2, 3, 2}, 3},
 	} {
 		t.Run("", func(t *testing.T) {
 			sequences = c.leaseSequences
@@ -1189,7 +1190,7 @@ func TestDistSenderIgnoresNLHEBasedOnOldRangeGeneration(t *testing.T) {
 			}
 			if tc.nlheLeaseSequence != 0 {
 				nlhe.Lease = &roachpb.Lease{
-					Sequence: roachpb.LeaseSequence(tc.nlheLeaseSequence),
+					Sequence: enginepb.LeaseSequence(tc.nlheLeaseSequence),
 					Replica:  roachpb.ReplicaDescriptor{NodeID: 4, StoreID: 4, ReplicaID: 4},
 				}
 			} else {
@@ -1202,7 +1203,7 @@ func TestDistSenderIgnoresNLHEBasedOnOldRangeGeneration(t *testing.T) {
 
 			cachedLease := roachpb.Lease{
 				Replica:  desc.InternalReplicas[1],
-				Sequence: roachpb.LeaseSequence(tc.cachedLeaseSequence),
+				Sequence: enginepb.LeaseSequence(tc.cachedLeaseSequence),
 			}
 
 			// The cache starts with a lease on node 2, so the first request will be
@@ -1415,7 +1416,7 @@ func TestDistSenderDownNodeEvictLeaseholder(t *testing.T) {
 			// The client has cleared the lease in the cache after the failure of the
 			// first RPC.
 			assert.Equal(t, desc.Generation, ba.ClientRangeInfo.DescriptorGeneration)
-			assert.Equal(t, roachpb.LeaseSequence(0), ba.ClientRangeInfo.LeaseSequence)
+			assert.Equal(t, enginepb.LeaseSequence(0), ba.ClientRangeInfo.LeaseSequence)
 			assert.Equal(t, roachpb.LEAD_FOR_GLOBAL_READS, ba.ClientRangeInfo.ClosedTimestampPolicy)
 			contacted2 = true
 			br := ba.CreateReply()

pkg/kv/kvclient/rangecache/BUILD.bazel

@@ -11,6 +11,7 @@ go_library(
         "//pkg/kv/kvpb",
         "//pkg/roachpb",
         "//pkg/settings/cluster",
+        "//pkg/storage/enginepb",
         "//pkg/util",
         "//pkg/util/cache",
         "//pkg/util/contextutil",
@@ -35,6 +36,7 @@ go_test(
         "//pkg/keys",
         "//pkg/roachpb",
         "//pkg/settings/cluster",
+        "//pkg/storage/enginepb",
         "//pkg/util/leaktest",
         "//pkg/util/log",
         "//pkg/util/stop",

pkg/kv/kvclient/rangecache/range_cache.go

@@ -24,6 +24,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
+	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util"
 	"github.com/cockroachdb/cockroach/pkg/util/cache"
 	"github.com/cockroachdb/cockroach/pkg/util/contextutil"
@@ -376,7 +377,7 @@ func (et EvictionToken) Lease() *roachpb.Lease {
 
 // LeaseSeq returns the sequence of the cached lease. If no lease is cached, or
 // the cached lease is speculative, 0 is returned.
-func (et EvictionToken) LeaseSeq() roachpb.LeaseSequence {
+func (et EvictionToken) LeaseSeq() enginepb.LeaseSequence {
 	if !et.Valid() {
 		panic("invalid LeaseSeq() call on empty EvictionToken")
 	}

pkg/kv/kvclient/rangecache/range_cache_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
+	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/stop"
@@ -1536,7 +1537,7 @@ func TestRangeCacheEvictAndReplace(t *testing.T) {
 	require.Equal(t, desc2, *tok.Desc())
 	require.NotNil(t, tok.Leaseholder())
 	require.Equal(t, rep1, *tok.Leaseholder())
-	require.Equal(t, roachpb.LeaseSequence(1), tok.LeaseSeq())
+	require.Equal(t, enginepb.LeaseSequence(1), tok.LeaseSeq())
 	require.Equal(t, lag, tok.ClosedTimestampPolicy(lead))
 
 	// EvictAndReplace() with a new closed timestamp policy.
@@ -1547,7 +1548,7 @@ func TestRangeCacheEvictAndReplace(t *testing.T) {
 	require.Equal(t, desc2, *tok.Desc())
 	require.NotNil(t, tok.Leaseholder())
 	require.Equal(t, rep1, *tok.Leaseholder())
-	require.Equal(t, roachpb.LeaseSequence(1), tok.LeaseSeq())
+	require.Equal(t, enginepb.LeaseSequence(1), tok.LeaseSeq())
 	require.Equal(t, lead, tok.ClosedTimestampPolicy(lag))
 
 	// EvictAndReplace() with a speculative descriptor. Should update decriptor,
@@ -1937,7 +1938,7 @@ func TestRangeCacheSyncTokenAndMaybeUpdateCache(t *testing.T) {
 				require.True(t, updatedLeaseholder)
 				require.Equal(t, &desc2, tok.Desc())
 				require.Equal(t, &rep2, tok.Leaseholder())
-				require.Equal(t, roachpb.LeaseSequence(0), tok.Lease().Sequence)
+				require.Equal(t, enginepb.LeaseSequence(0), tok.Lease().Sequence)
 			},
 		},
 	}
@@ -2151,7 +2152,7 @@ func TestRangeCacheEntryMaybeUpdate(t *testing.T) {
 		Replica:  rep1,
 		Sequence: 1,
 	}
-	require.Equal(t, roachpb.LeaseSequence(2), e.Lease().Sequence)
+	require.Equal(t, enginepb.LeaseSequence(2), e.Lease().Sequence)
 	updated, updatedLease, e = e.maybeUpdate(ctx, l, &desc3)
 	require.True(t, updated)
 	require.False(t, updatedLease)

pkg/kv/kvpb/api.proto

@@ -1462,7 +1462,7 @@ message TruncateLogRequest {
   RequestHeader header = 1 [(gogoproto.nullable) = false, (gogoproto.embed) = true];
 
   // Log entries < this index are to be discarded.
-  uint64 index = 2;
+  uint64 index = 2  [(gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/storage/enginepb.RaftIndex"];
 
   // RangeID is used to double check that the correct range is being truncated.
   // The header specifies a span, start and end keys, but not the range id
@@ -1482,7 +1482,7 @@ message TruncateLogRequest {
   // discarded from the raft log to be performed once, at the leaseholder.
   //
   // Populated starting at cluster version LooselyCoupledRaftLogTruncation.
-  uint64 expected_first_index = 4;
+  uint64 expected_first_index = 4  [(gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/storage/enginepb.RaftIndex"];
 }
 
 // TruncateLogResponse is the response to a TruncateLog() operation.
@@ -2099,7 +2099,7 @@ message SubsumeResponse {
   // provide the sender of the Subsume request with an upper bound on the lease
   // applied index of the CPut that left an intent on the local copy of the
   // right-hand range descriptor.
-  uint64 lease_applied_index = 4;
+  int64 lease_applied_index = 4  [(gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/storage/enginepb.LeaseSequence"];
 
   // FreezeStart is a timestamp that is guaranteed to be greater than the
   // timestamps at which any requests were serviced by the responding replica

pkg/kv/kvpb/errors.proto

@@ -325,7 +325,7 @@ message WriteIntentError {
   // operating under. Used on the server to avoid adding discovered locks
   // which were discovered under old leases to the lock table.
   optional int64 lease_sequence = 3 [(gogoproto.nullable) = false,
-    (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/roachpb.LeaseSequence"];
+    (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/storage/enginepb.LeaseSequence"];
   enum Reason {
     // The reason for the WriteIntentError is unspecified. This will
     // always be the case for errors returned from MVCC.

pkg/kv/kvserver/BUILD.bazel

@@ -125,7 +125,6 @@ go_library(
         "//pkg/kv/kvserver/batcheval",
         "//pkg/kv/kvserver/batcheval/result",
         "//pkg/kv/kvserver/closedts",
-        "//pkg/kv/kvserver/closedts/ctpb",
         "//pkg/kv/kvserver/closedts/sidetransport",
         "//pkg/kv/kvserver/closedts/tracker",
         "//pkg/kv/kvserver/concurrency",
@@ -364,7 +363,6 @@ go_test(
         "//pkg/kv/kvserver/batcheval",
         "//pkg/kv/kvserver/batcheval/result",
         "//pkg/kv/kvserver/closedts",
-        "//pkg/kv/kvserver/closedts/ctpb",
         "//pkg/kv/kvserver/closedts/tracker",
         "//pkg/kv/kvserver/concurrency",
         "//pkg/kv/kvserver/concurrency/lock",

pkg/kv/kvserver/allocator/allocatorimpl/BUILD.bazel

@@ -22,6 +22,7 @@ go_library(
         "//pkg/roachpb",
         "//pkg/settings",
         "//pkg/settings/cluster",
+        "//pkg/storage/enginepb",
         "//pkg/util/admission/admissionpb",
         "//pkg/util/log",
         "//pkg/util/metric",
@@ -54,6 +55,7 @@ go_test(
         "//pkg/kv/kvserver/replicastats",
         "//pkg/roachpb",
         "//pkg/settings/cluster",
+        "//pkg/storage/enginepb",
         "//pkg/testutils",
         "//pkg/testutils/gossiputil",
         "//pkg/util",

pkg/kv/kvserver/allocator/allocatorimpl/allocator.go

@@ -27,6 +27,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
+	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/metric"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
@@ -1957,7 +1958,7 @@ func (a *Allocator) ValidLeaseTargets(
 	leaseRepl interface {
 		StoreID() roachpb.StoreID
 		RaftStatus() *raft.Status
-		GetFirstIndex() uint64
+		GetFirstIndex() enginepb.RaftIndex
 		Desc() *roachpb.RangeDescriptor
 	},
 	opts allocator.TransferLeaseOptions,
@@ -2132,7 +2133,7 @@ func (a *Allocator) leaseholderShouldMoveDueToPreferences(
 	leaseRepl interface {
 		StoreID() roachpb.StoreID
 		RaftStatus() *raft.Status
-		GetFirstIndex() uint64
+		GetFirstIndex() enginepb.RaftIndex
 	},
 	allExistingReplicas []roachpb.ReplicaDescriptor,
 ) bool {
@@ -2217,7 +2218,7 @@ func (a *Allocator) TransferLeaseTarget(
 		StoreID() roachpb.StoreID
 		GetRangeID() roachpb.RangeID
 		RaftStatus() *raft.Status
-		GetFirstIndex() uint64
+		GetFirstIndex() enginepb.RaftIndex
 		Desc() *roachpb.RangeDescriptor
 	},
 	usageInfo allocator.RangeUsageInfo,
@@ -2485,7 +2486,7 @@ func (a *Allocator) ShouldTransferLease(
 	leaseRepl interface {
 		StoreID() roachpb.StoreID
 		RaftStatus() *raft.Status
-		GetFirstIndex() uint64
+		GetFirstIndex() enginepb.RaftIndex
 		Desc() *roachpb.RangeDescriptor
 	},
 	usageInfo allocator.RangeUsageInfo,
@@ -2852,7 +2853,10 @@ func FilterBehindReplicas(
 // Other replicas may be filtered out if this function is called with the
 // `raftStatus` of a non-raft leader replica.
 func excludeReplicasInNeedOfSnapshots(
-	ctx context.Context, st *raft.Status, firstIndex uint64, replicas []roachpb.ReplicaDescriptor,
+	ctx context.Context,
+	st *raft.Status,
+	firstIndex enginepb.RaftIndex,
+	replicas []roachpb.ReplicaDescriptor,
 ) []roachpb.ReplicaDescriptor {
 	filled := 0
 	for _, repl := range replicas {