diff --git a/pkg/kv/kvnemesis/kvnemesis_test.go b/pkg/kv/kvnemesis/kvnemesis_test.go index 151597b44923..389afedd0ba5 100644 --- a/pkg/kv/kvnemesis/kvnemesis_test.go +++ b/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, ) { diff --git a/pkg/kv/kvserver/apply/asserter.go b/pkg/kv/kvserver/apply/asserter.go index 73e4200dfbb4..0a7f3180a5f6 100644 --- a/pkg/kv/kvserver/apply/asserter.go +++ b/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 } diff --git a/pkg/kv/kvserver/store_split.go b/pkg/kv/kvserver/store_split.go index d6e3d3e02801..ce9cde8565fb 100644 --- a/pkg/kv/kvserver/store_split.go +++ b/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 } diff --git a/pkg/kv/kvserver/testing_knobs.go b/pkg/kv/kvserver/testing_knobs.go index 83c48526e94a..22953f52b093 100644 --- a/pkg/kv/kvserver/testing_knobs.go +++ b/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)