Merge pull request #159213 from cockroachdb/blathers/backport-release-26.1-158349

release-26.1: kvnemesis: report splits to raft asserter

Author: pav-kv

pkg/kv/kvnemesis/kvnemesis_test.go

@@ -170,6 +170,13 @@ func (cfg kvnemesisTestCfg) testClusterArgs(
 			}
 			return 0, nil
 		}
+		storeKnobs.AfterSplitApplication = func(desc roachpb.ReplicaDescriptor, state kvserverpb.ReplicaState) {
+			asserter.ApplySplitRHS(
+				state.Desc.RangeID, desc.ReplicaID,
+				state.RaftAppliedIndex, state.RaftAppliedIndexTerm,
+				state.LeaseAppliedIndex, state.RaftClosedTimestamp,
+			)
+		}
 		storeKnobs.AfterSnapshotApplication = func(
 			desc roachpb.ReplicaDescriptor, state kvserverpb.ReplicaState, snap kvserver.IncomingSnapshot,
 		) {

pkg/kv/kvserver/apply/asserter.go

@@ -342,6 +342,33 @@ func (r *rangeAsserter) apply(
 	r.seedAppliedAs[seedID] = cmdID
 }
 
+// ApplySplitRHS tracks and asserts command applications which split out a new
+// range. Accepts the state of the RHS.
+func (a *Asserter) ApplySplitRHS(
+	rangeID roachpb.RangeID,
+	replicaID roachpb.ReplicaID,
+	raftAppliedIndex kvpb.RaftIndex,
+	raftAppliedIndexTerm kvpb.RaftTerm,
+	leaseAppliedIndex kvpb.LeaseAppliedIndex,
+	closedTS hlc.Timestamp,
+) {
+	// Use a special command ID that signifies a pseudo-proposal that splits this
+	// range out. This ID does not match any "real" IDs (they are of a different
+	// size, and sufficiently random even if were of the same size).
+	const cmdID = kvserverbase.CmdIDKey("__split__")
+	r := a.forRange(rangeID)
+	// Circumvent the check in r.apply that the command ID must be proposed.
+	func() {
+		r.Lock()
+		defer r.Unlock()
+		r.proposedCmds[cmdID] = 0
+	}()
+	// Pretend that we are applying this pseudo-command.
+	r.apply(replicaID, cmdID, raftpb.Entry{
+		Index: uint64(raftAppliedIndex), Term: uint64(raftAppliedIndexTerm),
+	}, leaseAppliedIndex, closedTS)
+}
+
 // ApplySnapshot tracks and asserts snapshot application.
 func (a *Asserter) ApplySnapshot(
 	rangeID roachpb.RangeID,
@@ -385,13 +412,22 @@ func (r *rangeAsserter) applySnapshot(
 	}
 	r.replicaAppliedIndex[replicaID] = index
 
-	// We can't have a snapshot without any applied log entries, except when this
-	// is an initial snapshot. It's possible that the initial snapshot follows an
-	// empty entry appended by the raft leader at the start of this term. Since we
-	// don't register this entry as applied, r.log can be empty here.
+	// Typically, we can't have a snapshot without any applied entries. For most
+	// ranges, there is at least a pseudo-command (see ApplySplitRHS) that created
+	// this range as part of a split.
+	//
+	// For ranges that were not a result of a split (those created during the
+	// cluster bootstrap), the only way to see an empty r.log here is that this
+	// snapshot follows only empty entries appended by the raft leader at the
+	// start of its term or other no-op entries (see comment in r.apply explaining
+	// that we do not register such commands).
+	//
+	// The latter situation appears impossible, because the bootstrapped ranges
+	// can't send a snapshot to anyone without applying a config change adding
+	// another replica - so r.log can't be empty.
 	//
-	// See the comment in r.apply() method, around the empty cmdID check, and the
-	// comment for r.log saying that there can be gaps in the observed applies.
+	// TODO(pav-kv): consider dropping this condition, or reporting the initial
+	// state during the cluster bootstrap for completeness.
 	if len(r.log) == 0 {
 		return
 	}

pkg/kv/kvserver/store_split.go

@@ -313,6 +313,15 @@ func prepareRightReplicaForSplit(
 	// Raft group, there might not be any Raft traffic for quite a while.
 	rightRepl.maybeUnquiesceLocked(true /* wakeLeader */, true /* mayCampaign */)
 
+	if fn := r.store.TestingKnobs().AfterSplitApplication; fn != nil {
+		// TODO(pav-kv): we have already checked up the stack that rightDesc exists,
+		// but maybe it would be better to carry a "bus" struct with these kinds of
+		// data post checks so that we (a) don't need to repeat the computation /
+		// validation, (b) can be sure that it's consistent.
+		rightDesc, _ := split.RightDesc.GetReplicaDescriptor(r.StoreID())
+		fn(rightDesc, rightRepl.shMu.state)
+	}
+
 	return rightRepl
 }
 

pkg/kv/kvserver/testing_knobs.go

@@ -379,6 +379,9 @@ type StoreTestingKnobs struct {
 	// BeforeSnapshotSSTIngestion is run just before the SSTs are ingested when
 	// applying a snapshot.
 	BeforeSnapshotSSTIngestion func(IncomingSnapshot, []string) error
+	// AfterSplitApplication is called on the newly created replica's state after
+	// a split is applied. Called iff the RHS replica is not already destroyed.
+	AfterSplitApplication func(roachpb.ReplicaDescriptor, kvserverpb.ReplicaState)
 	// AfterSnapshotApplication is run after a snapshot is applied, before
 	// releasing the replica mutex.
 	AfterSnapshotApplication func(roachpb.ReplicaDescriptor, kvserverpb.ReplicaState, IncomingSnapshot)