Merge #155565

155565: kvstorage: introduce state/raft storage types r=arulajmani a=pav-kv

This PR introduces basic type aliases for raft and state machine engines, and various combinations of them that are used in the code as of today. These will evolve as we continue refactoring `kvstorage` package and its users.

This PR also merges the `kvserver/stateloader` package into `kvserver/kvstorage`. For 2 reasons: (a) avoid circular dependency, (b) the stateloader code logically belongs to and is tightly integrated with `kvstorage`.

Part of #97618

Co-authored-by: Pavel Kalinnikov <[email protected]>

Author: craig[bot]

pkg/BUILD.bazel

@@ -298,7 +298,6 @@ ALL_TESTS = [
     "//pkg/kv/kvserver/spanlatch:spanlatch_test",
     "//pkg/kv/kvserver/spanset:spanset_test",
     "//pkg/kv/kvserver/split:split_test",
-    "//pkg/kv/kvserver/stateloader:stateloader_test",
     "//pkg/kv/kvserver/storeliveness:storeliveness_test",
     "//pkg/kv/kvserver/tenantrate:tenantrate_test",
     "//pkg/kv/kvserver/tscache:tscache_test",
@@ -1644,8 +1643,6 @@ GO_TARGETS = [
     "//pkg/kv/kvserver/spanset:spanset_test",
     "//pkg/kv/kvserver/split:split",
     "//pkg/kv/kvserver/split:split_test",
-    "//pkg/kv/kvserver/stateloader:stateloader",
-    "//pkg/kv/kvserver/stateloader:stateloader_test",
     "//pkg/kv/kvserver/storeliveness/storelivenesspb:storelivenesspb",
     "//pkg/kv/kvserver/storeliveness:storeliveness",
     "//pkg/kv/kvserver/storeliveness:storeliveness_test",

pkg/cli/BUILD.bazel

@@ -137,7 +137,6 @@ go_library(
         "//pkg/kv/kvserver/loqrecovery/loqrecoverypb",
         "//pkg/kv/kvserver/print",
         "//pkg/kv/kvserver/rditer",
-        "//pkg/kv/kvserver/stateloader",
         "//pkg/multitenant/mtinfopb",
         "//pkg/raft/raftpb",
         "//pkg/roachpb",
@@ -421,11 +420,11 @@ go_test(
         "//pkg/kv/kvclient/kvtenant",
         "//pkg/kv/kvpb",
         "//pkg/kv/kvserver",
+        "//pkg/kv/kvserver/kvstorage",
         "//pkg/kv/kvserver/liveness",
         "//pkg/kv/kvserver/liveness/livenesspb",
         "//pkg/kv/kvserver/loqrecovery",
         "//pkg/kv/kvserver/loqrecovery/loqrecoverypb",
-        "//pkg/kv/kvserver/stateloader",
         "//pkg/roachpb",
         "//pkg/security/clientsecopts",
         "//pkg/security/securityassets",

pkg/cli/debug_check_store.go

@@ -18,7 +18,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvstorage"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/rditer"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
 	"github.com/cockroachdb/cockroach/pkg/raft/raftpb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/storage"
@@ -88,7 +87,7 @@ type replicaCheckInfo struct {
 type checkInput struct {
 	eng  storage.Engine
 	desc *roachpb.RangeDescriptor
-	sl   stateloader.StateLoader
+	sl   kvstorage.StateLoader
 }
 
 type checkResult struct {
@@ -166,7 +165,7 @@ func checkStoreRangeStats(
 	go func() {
 		if err := kvstorage.IterateRangeDescriptorsFromDisk(ctx, eng,
 			func(desc roachpb.RangeDescriptor) error {
-				inCh <- checkInput{eng: eng, desc: &desc, sl: stateloader.Make(desc.RangeID)}
+				inCh <- checkInput{eng: eng, desc: &desc, sl: kvstorage.MakeStateLoader(desc.RangeID)}
 				return nil
 			}); err != nil {
 			outCh <- checkResult{err: err}

pkg/cli/debug_check_store_test.go

@@ -14,7 +14,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvstorage"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/storage"
@@ -95,7 +95,7 @@ func TestDebugCheckStore(t *testing.T) {
 			storage.MustExist)
 		require.NoError(t, err)
 		defer eng.Close()
-		sl := stateloader.Make(1)
+		sl := kvstorage.MakeStateLoader(1)
 		ms, err := sl.LoadMVCCStats(ctx, eng)
 		require.NoError(t, err)
 		ms.ContainsEstimates = 0

pkg/kv/kvserver/BUILD.bazel

@@ -181,7 +181,6 @@ go_library(
         "//pkg/kv/kvserver/replicastats",
         "//pkg/kv/kvserver/spanset",
         "//pkg/kv/kvserver/split",
-        "//pkg/kv/kvserver/stateloader",
         "//pkg/kv/kvserver/storeliveness",
         "//pkg/kv/kvserver/storeliveness/storelivenesspb",
         "//pkg/kv/kvserver/tenantrate",
@@ -470,7 +469,6 @@ go_test(
         "//pkg/kv/kvserver/replicastats",
         "//pkg/kv/kvserver/spanset",
         "//pkg/kv/kvserver/split",
-        "//pkg/kv/kvserver/stateloader",
         "//pkg/kv/kvserver/storeliveness",
         "//pkg/kv/kvserver/storeliveness/storelivenesspb",
         "//pkg/kv/kvserver/tenantrate",

pkg/kv/kvserver/batcheval/BUILD.bazel

@@ -70,12 +70,12 @@ go_library(
         "//pkg/kv/kvserver/intentresolver",
         "//pkg/kv/kvserver/kvserverbase",
         "//pkg/kv/kvserver/kvserverpb",
+        "//pkg/kv/kvserver/kvstorage",
         "//pkg/kv/kvserver/lockspanset",
         "//pkg/kv/kvserver/rditer",
         "//pkg/kv/kvserver/readsummary",
         "//pkg/kv/kvserver/readsummary/rspb",
         "//pkg/kv/kvserver/spanset",
-        "//pkg/kv/kvserver/stateloader",
         "//pkg/kv/kvserver/txnwait",
         "//pkg/kv/kvserver/uncertainty",
         "//pkg/roachpb",
@@ -156,11 +156,11 @@ go_test(
         "//pkg/kv/kvserver/gc",
         "//pkg/kv/kvserver/kvserverbase",
         "//pkg/kv/kvserver/kvserverpb",
+        "//pkg/kv/kvserver/kvstorage",
         "//pkg/kv/kvserver/lockspanset",
         "//pkg/kv/kvserver/readsummary",
         "//pkg/kv/kvserver/readsummary/rspb",
         "//pkg/kv/kvserver/spanset",
-        "//pkg/kv/kvserver/stateloader",
         "//pkg/kv/kvserver/txnwait",
         "//pkg/roachpb",
         "//pkg/security/securityassets",

pkg/kv/kvserver/batcheval/cmd_end_transaction.go

@@ -19,11 +19,11 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/gc"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvstorage"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/lockspanset"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/rditer"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/readsummary"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/spanset"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/storage"
@@ -1511,7 +1511,7 @@ func splitTriggerHelper(
 		// HardState via a call to synthesizeRaftState. Here, we only call
 		// writeInitialReplicaState which essentially writes a ReplicaState
 		// only.
-		if *h.AbsPostSplitRight(), err = stateloader.WriteInitialReplicaState(
+		if *h.AbsPostSplitRight(), err = kvstorage.WriteInitialReplicaState(
 			ctx, batch, *h.AbsPostSplitRight(), split.RightDesc, rightLease,
 			*in.GCThreshold, *in.GCHint, in.ReplicaVersion,
 		); err != nil {
@@ -1523,7 +1523,7 @@ func splitTriggerHelper(
 		// as all replicas will be responsible for writing it locally before
 		// applying the split.
 		if !rec.ClusterSettings().Version.IsActive(ctx, clusterversion.V25_4_WriteInitialTruncStateBeforeSplitApplication) {
-			if err := stateloader.WriteInitialTruncState(ctx, batch, split.RightDesc.RangeID); err != nil {
+			if err := kvstorage.WriteInitialTruncState(ctx, batch, split.RightDesc.RangeID); err != nil {
 				return enginepb.MVCCStats{}, result.Result{}, errors.Wrap(err, "unable to write initial Replica state")
 			}
 		}
@@ -1655,7 +1655,7 @@ func mergeTrigger(
 		if err != nil {
 			return result.Result{}, err
 		}
-		rhsLoader := stateloader.Make(merge.RightDesc.RangeID)
+		rhsLoader := kvstorage.MakeStateLoader(merge.RightDesc.RangeID)
 		rhsHint, err := rhsLoader.LoadGCHint(ctx, batch)
 		if err != nil {
 			return result.Result{}, err

pkg/kv/kvserver/batcheval/cmd_end_transaction_test.go

@@ -19,7 +19,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/isolation"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvstorage"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/storage"
@@ -2169,7 +2169,7 @@ func TestSplitTriggerWritesInitialReplicaState(t *testing.T) {
 	gcHint := roachpb.GCHint{GCTimestamp: gcThreshold}
 	abortSpanTxnID := uuid.MakeV4()
 	as := abortspan.New(desc.RangeID)
-	sl := stateloader.Make(desc.RangeID)
+	sl := kvstorage.MakeStateLoader(desc.RangeID)
 	rec := (&MockEvalCtx{
 		ClusterSettings:        st,
 		Desc:                   &desc,
@@ -2217,7 +2217,7 @@ func TestSplitTriggerWritesInitialReplicaState(t *testing.T) {
 
 	// Verify that range state was migrated to the right-hand side properly.
 	asRight := abortspan.New(rightDesc.RangeID)
-	slRight := stateloader.Make(rightDesc.RangeID)
+	slRight := kvstorage.MakeStateLoader(rightDesc.RangeID)
 	// The abort span should have been transferred over.
 	ok, err := asRight.Get(ctx, batch, abortSpanTxnID, &roachpb.AbortSpanEntry{})
 	require.NoError(t, err)
@@ -2251,9 +2251,9 @@ func TestSplitTriggerWritesInitialReplicaState(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, version, loadedVersion)
 	expAppliedState := kvserverpb.RangeAppliedState{
-		RaftAppliedIndexTerm: stateloader.RaftInitialLogTerm,
-		RaftAppliedIndex:     stateloader.RaftInitialLogIndex,
-		LeaseAppliedIndex:    stateloader.InitialLeaseAppliedIndex,
+		RaftAppliedIndexTerm: kvstorage.RaftInitialLogTerm,
+		RaftAppliedIndex:     kvstorage.RaftInitialLogIndex,
+		LeaseAppliedIndex:    kvstorage.InitialLeaseAppliedIndex,
 	}
 	loadedAppliedState, err := slRight.LoadRangeAppliedState(ctx, batch)
 	require.NoError(t, err)

pkg/kv/kvserver/batcheval/stateloader.go

@@ -5,9 +5,9 @@
 
 package batcheval
 
-import "github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
+import "github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvstorage"
 
 // MakeStateLoader creates a StateLoader for the EvalContext.
-func MakeStateLoader(rec EvalContext) stateloader.StateLoader {
-	return stateloader.Make(rec.GetRangeID())
+func MakeStateLoader(rec EvalContext) kvstorage.StateLoader {
+	return kvstorage.MakeStateLoader(rec.GetRangeID())
 }

pkg/kv/kvserver/client_manual_proposal_test.go

@@ -18,9 +18,9 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvstorage"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/logstore"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/raftlog"
-	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/uncertainty"
 	"github.com/cockroachdb/cockroach/pkg/raft"
 	"github.com/cockroachdb/cockroach/pkg/raft/raftpb"
@@ -137,7 +137,7 @@ LIMIT
 			return nil
 		}))
 
-	sl := stateloader.Make(rangeID)
+	sl := kvstorage.MakeStateLoader(rangeID)
 	lease, err := sl.LoadLease(ctx, eng)
 	require.NoError(t, err)