Skip to content

Commit 9587fd6

Browse files
committed
kvnemesis: report splits to raft asserter
This commit passes the information about newly created replicas into the raft asserter. This helps tracking the applied state more accurately. Previously, there was a window of time (between a replica splits out and applies the first non-trivial command) during which its tracked applied state is zero while the real one is not. Epic: none Release note: none
1 parent 190c04f commit 9587fd6

File tree

2 files changed

+47
-7
lines changed

2 files changed

+47
-7
lines changed

pkg/kv/kvnemesis/kvnemesis_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,11 @@ func (cfg kvnemesisTestCfg) testClusterArgs(
171171
return 0, nil
172172
}
173173
storeKnobs.AfterSplitApplication = func(desc roachpb.ReplicaDescriptor, state kvserverpb.ReplicaState) {
174-
// TODO(pav-kv): notify the asserter.
174+
asserter.ApplySplitRHS(
175+
state.Desc.RangeID, desc.ReplicaID,
176+
state.RaftAppliedIndex, state.RaftAppliedIndexTerm,
177+
state.LeaseAppliedIndex, state.RaftClosedTimestamp,
178+
)
175179
}
176180
storeKnobs.AfterSnapshotApplication = func(
177181
desc roachpb.ReplicaDescriptor, state kvserverpb.ReplicaState, snap kvserver.IncomingSnapshot,

pkg/kv/kvserver/apply/asserter.go

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,33 @@ func (r *rangeAsserter) apply(
342342
r.seedAppliedAs[seedID] = cmdID
343343
}
344344

345+
// ApplySplitRHS tracks and asserts command applications which split out a new
346+
// range. Accepts the state of the RHS.
347+
func (a *Asserter) ApplySplitRHS(
348+
rangeID roachpb.RangeID,
349+
replicaID roachpb.ReplicaID,
350+
raftAppliedIndex kvpb.RaftIndex,
351+
raftAppliedIndexTerm kvpb.RaftTerm,
352+
leaseAppliedIndex kvpb.LeaseAppliedIndex,
353+
closedTS hlc.Timestamp,
354+
) {
355+
// Use a special command ID that signifies a pseudo-proposal that splits this
356+
// range out. This ID does not match any "real" IDs (they are of a different
357+
// size, and sufficiently random even if were of the same size).
358+
const cmdID = kvserverbase.CmdIDKey("__split__")
359+
r := a.forRange(rangeID)
360+
// Circumvent the check in r.apply that the command ID must be proposed.
361+
func() {
362+
r.Lock()
363+
defer r.Unlock()
364+
r.proposedCmds[cmdID] = 0
365+
}()
366+
// Pretend that we are applying this pseudo-command.
367+
r.apply(replicaID, cmdID, raftpb.Entry{
368+
Index: uint64(raftAppliedIndex), Term: uint64(raftAppliedIndexTerm),
369+
}, leaseAppliedIndex, closedTS)
370+
}
371+
345372
// ApplySnapshot tracks and asserts snapshot application.
346373
func (a *Asserter) ApplySnapshot(
347374
rangeID roachpb.RangeID,
@@ -385,13 +412,22 @@ func (r *rangeAsserter) applySnapshot(
385412
}
386413
r.replicaAppliedIndex[replicaID] = index
387414

388-
// We can't have a snapshot without any applied log entries, except when this
389-
// is an initial snapshot. It's possible that the initial snapshot follows an
390-
// empty entry appended by the raft leader at the start of this term. Since we
391-
// don't register this entry as applied, r.log can be empty here.
415+
// Typically, we can't have a snapshot without any applied entries. For most
416+
// ranges, there is at least a pseudo-command (see ApplySplitRHS) that created
417+
// this range as part of a split.
418+
//
419+
// For ranges that were not a result of a split (those created during the
420+
// cluster bootstrap), the only way to see an empty r.log here is that this
421+
// snapshot follows only empty entries appended by the raft leader at the
422+
// start of its term or other no-op entries (see comment in r.apply explaining
423+
// that we do not register such commands).
424+
//
425+
// The latter situation appears impossible, because the bootstrapped ranges
426+
// can't send a snapshot to anyone without applying a config change adding
427+
// another replica - so r.log can't be empty.
392428
//
393-
// See the comment in r.apply() method, around the empty cmdID check, and the
394-
// comment for r.log saying that there can be gaps in the observed applies.
429+
// TODO(pav-kv): consider dropping this condition, or reporting the initial
430+
// state during the cluster bootstrap for completeness.
395431
if len(r.log) == 0 {
396432
return
397433
}

0 commit comments

Comments
 (0)