Merge #147218

147218: kvserver: rm unnecessary ReplicaID write r=arulajmani,tbg a=pav-kv

In `splitPreApply`, the common case is that the replica exists because it was created by `acquireSplitLock` which calls `getOrCreateReplica`, which ultimately writes the `ReplicaID`. Alternatively, this replica could have been brought to existence by an earlier `getOrCreateReplica`. We don't need to duplicate the `ReplicaID` write in this case.

Another way to understand this: a replica **always** goes through the uninitialized state before being initialized. We already write `ReplicaID` when going through the uninitialized state.

Epic: CRDB-46488

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

Author: craig[bot]

pkg/kv/kvserver/store_split.go

@@ -38,7 +38,7 @@ func splitPreApply(
 	//
 	// The exception to that is if the DisableEagerReplicaRemoval testing flag is
 	// enabled.
-	rightDesc, hasRightDesc := split.RightDesc.GetReplicaDescriptor(r.StoreID())
+	_, hasRightDesc := split.RightDesc.GetReplicaDescriptor(r.StoreID())
 	_, hasLeftDesc := split.LeftDesc.GetReplicaDescriptor(r.StoreID())
 	if !hasRightDesc || !hasLeftDesc {
 		log.Fatalf(ctx, "cannot process split on s%s which does not exist in the split: %+v",
@@ -130,21 +130,14 @@ func splitPreApply(
 		return
 	}
 
-	// Update the raft HardState with the new Commit value now that the
-	// replica is initialized (combining it with existing or default
-	// Term and Vote). This is the common case.
+	// The RHS replica exists and is uninitialized. We are initializing it here.
+	// Update the raft HardState with the new Commit index (taken from the applied
+	// state in the write batch), and use existing or default Term and Vote. This
+	// is the common case.
 	rsl := stateloader.Make(split.RightDesc.RangeID)
 	if err := rsl.SynthesizeRaftState(ctx, readWriter); err != nil {
 		log.Fatalf(ctx, "%v", err)
 	}
-	// Write the RaftReplicaID for the RHS to maintain the invariant that any
-	// replica (uninitialized or initialized), with persistent state, has a
-	// RaftReplicaID. NB: this invariant will not be universally true until we
-	// introduce node startup code that will write this value for existing
-	// ranges.
-	if err := rsl.SetRaftReplicaID(ctx, readWriter, rightDesc.ReplicaID); err != nil {
-		log.Fatalf(ctx, "%v", err)
-	}
 	// Persist the closed timestamp.
 	//
 	// In order to tolerate a nil initClosedTS input, let's forward to