Merge #155408

155408: kvserver: refactors to enable replica lifecycle testing r=pav-kv a=arulajmani

See individual commits.

Co-authored-by: Arul Ajmani <arulajmani@gmail.com>

Author: craig[bot]

pkg/kv/kvserver/batcheval/cmd_end_transaction.go

@@ -930,11 +930,11 @@ func RunCommitTrigger(
 				ctx, errors.Wrap(err, "unable to load replica version"),
 			)
 		}
-		in := splitTriggerHelperInput{
-			leftLease:      lhsLease,
-			gcThreshold:    gcThreshold,
-			gcHint:         gcHint,
-			replicaVersion: replicaVersion,
+		in := SplitTriggerHelperInput{
+			LeftLease:      lhsLease,
+			GCThreshold:    gcThreshold,
+			GCHint:         gcHint,
+			ReplicaVersion: replicaVersion,
 		}
 
 		newMS, res, err := splitTrigger(
@@ -1164,7 +1164,7 @@ func splitTrigger(
 	batch storage.Batch,
 	bothDeltaMS enginepb.MVCCStats,
 	split *roachpb.SplitTrigger,
-	in splitTriggerHelperInput,
+	in SplitTriggerHelperInput,
 	ts hlc.Timestamp,
 ) (enginepb.MVCCStats, result.Result, error) {
 	desc := rec.Desc()
@@ -1243,6 +1243,20 @@ func splitTrigger(
 	return splitTriggerHelper(ctx, rec, batch, in, h, split, ts)
 }
 
+// TestingSplitTrigger is a wrapper around splitTrigger that is exported for
+// testing purposes.
+func TestingSplitTrigger(
+	ctx context.Context,
+	rec EvalContext,
+	batch storage.Batch,
+	bothDeltaMS enginepb.MVCCStats,
+	split *roachpb.SplitTrigger,
+	in SplitTriggerHelperInput,
+	ts hlc.Timestamp,
+) (enginepb.MVCCStats, result.Result, error) {
+	return splitTrigger(ctx, rec, batch, bothDeltaMS, split, in, ts)
+}
+
 // splitScansRightForStatsFirst controls whether the left hand side or the right
 // hand side of the split is scanned first on the leaseholder when evaluating
 // the split trigger. In practice, the splitQueue wants to scan the left hand
@@ -1276,13 +1290,13 @@ func makeScanStatsFn(
 	}
 }
 
-// splitTriggerHelperInput contains metadata needed by the RHS when running the
+// SplitTriggerHelperInput contains metadata needed by the RHS when running the
 // splitTriggerHelper.
-type splitTriggerHelperInput struct {
-	leftLease      roachpb.Lease
-	gcThreshold    *hlc.Timestamp
-	gcHint         *roachpb.GCHint
-	replicaVersion roachpb.Version
+type SplitTriggerHelperInput struct {
+	LeftLease      roachpb.Lease
+	GCThreshold    *hlc.Timestamp
+	GCHint         *roachpb.GCHint
+	ReplicaVersion roachpb.Version
 }
 
 // splitTriggerHelper continues the work begun by splitTrigger, but has a
@@ -1292,7 +1306,7 @@ func splitTriggerHelper(
 	ctx context.Context,
 	rec EvalContext,
 	batch storage.Batch,
-	in splitTriggerHelperInput,
+	in SplitTriggerHelperInput,
 	statsInput splitStatsHelperInput,
 	split *roachpb.SplitTrigger,
 	ts hlc.Timestamp,
@@ -1434,22 +1448,22 @@ func splitTriggerHelper(
 		// - node two becomes the lease holder for [c,e). Its timestamp cache does
 		//   not know about the read at 'd' which happened at the beginning.
 		// - node two can illegally propose a write to 'd' at a lower timestamp.
-		if in.leftLease.Empty() {
+		if in.LeftLease.Empty() {
 			log.KvExec.Fatalf(ctx, "LHS of split has no lease")
 		}
 
 		// Copy the lease from the left-hand side of the split over to the
 		// right-hand side so that it can immediately start serving requests.
 		// When doing so, we need to make a few modifications.
-		rightLease := in.leftLease
+		rightLease := in.LeftLease
 		// Rebind the lease to the existing leaseholder store's replica from the
 		// right-hand side's descriptor.
 		var ok bool
-		rightLease.Replica, ok = split.RightDesc.GetReplicaDescriptor(in.leftLease.Replica.StoreID)
+		rightLease.Replica, ok = split.RightDesc.GetReplicaDescriptor(in.LeftLease.Replica.StoreID)
 		if !ok {
 			return enginepb.MVCCStats{}, result.Result{}, errors.Errorf(
 				"pre-split lease holder %+v not found in post-split descriptor %+v",
-				in.leftLease.Replica, split.RightDesc,
+				in.LeftLease.Replica, split.RightDesc,
 			)
 		}
 		// Convert leader leases into expiration-based leases. A leader lease is
@@ -1464,7 +1478,7 @@ func splitTriggerHelper(
 			rightLease.Term = 0
 			rightLease.MinExpiration = hlc.Timestamp{}
 		}
-		if in.gcThreshold.IsEmpty() {
+		if in.GCThreshold.IsEmpty() {
 			log.VEventf(ctx, 1, "LHS's GCThreshold of split is not set")
 		}
 
@@ -1499,7 +1513,7 @@ func splitTriggerHelper(
 		// only.
 		if *h.AbsPostSplitRight(), err = stateloader.WriteInitialReplicaState(
 			ctx, batch, *h.AbsPostSplitRight(), split.RightDesc, rightLease,
-			*in.gcThreshold, *in.gcHint, in.replicaVersion,
+			*in.GCThreshold, *in.GCHint, in.ReplicaVersion,
 		); err != nil {
 			return enginepb.MVCCStats{}, result.Result{}, errors.Wrap(err, "unable to write initial Replica state")
 		}

pkg/kv/kvserver/batcheval/cmd_end_transaction_test.go

@@ -2203,11 +2203,11 @@ func TestSplitTriggerWritesInitialReplicaState(t *testing.T) {
 	err = sl.SetVersion(ctx, batch, nil, &version)
 	require.NoError(t, err)
 
-	in := splitTriggerHelperInput{
-		leftLease:      lease,
-		gcThreshold:    &gcThreshold,
-		gcHint:         &gcHint,
-		replicaVersion: version,
+	in := SplitTriggerHelperInput{
+		LeftLease:      lease,
+		GCThreshold:    &gcThreshold,
+		GCHint:         &gcHint,
+		ReplicaVersion: version,
 	}
 
 	// Run the split trigger, which is normally run as a subset of EndTxn request

pkg/kv/kvserver/kvstorage/replica_state.go

@@ -120,14 +120,18 @@ const CreateUninitReplicaTODO = 0
 // Returns kvpb.RaftGroupDeletedError if this replica can not be created
 // because it has been deleted.
 func CreateUninitializedReplica(
-	ctx context.Context, eng storage.Engine, storeID roachpb.StoreID, id roachpb.FullReplicaID,
+	ctx context.Context,
+	reader storage.Reader,
+	writer storage.Writer,
+	storeID roachpb.StoreID,
+	id roachpb.FullReplicaID,
 ) error {
 	sl := stateloader.Make(id.RangeID)
 	// Before creating the replica, see if there is a tombstone which would
 	// indicate that this replica has been removed.
 	// TODO(pav-kv): should also check that there is no existing replica, i.e.
 	// ReplicaID load should find nothing.
-	if ts, err := sl.LoadRangeTombstone(ctx, eng); err != nil {
+	if ts, err := sl.LoadRangeTombstone(ctx, reader); err != nil {
 		return err
 	} else if id.ReplicaID < ts.NextReplicaID {
 		return &kvpb.RaftGroupDeletedError{}
@@ -140,12 +144,12 @@ func CreateUninitializedReplica(
 	// non-existent. The only RangeID-specific key that can be present is the
 	// RangeTombstone inspected above.
 	_ = CreateUninitReplicaTODO
-	if err := sl.SetRaftReplicaID(ctx, eng, id.ReplicaID); err != nil {
+	if err := sl.SetRaftReplicaID(ctx, writer, id.ReplicaID); err != nil {
 		return err
 	}
 
 	// Make sure that storage invariants for this uninitialized replica hold.
 	uninitDesc := roachpb.RangeDescriptor{RangeID: id.RangeID}
-	_, err := LoadReplicaState(ctx, eng, storeID, &uninitDesc, id.ReplicaID)
+	_, err := LoadReplicaState(ctx, reader, storeID, &uninitDesc, id.ReplicaID)
 	return err
 }

pkg/kv/kvserver/store_create_replica.go

@@ -187,7 +187,7 @@ func (s *Store) tryGetOrCreateReplica(
 	// TODO(sep-raft-log): needs both engines due to tombstone (which lives on
 	// statemachine).
 	if err := kvstorage.CreateUninitializedReplica(
-		ctx, s.TODOEngine(), s.StoreID(), id,
+		ctx, s.TODOEngine(), s.TODOEngine(), s.StoreID(), id,
 	); err != nil {
 		return nil, false, err
 	}