storage: delay split that would result in more snapshots

When a Range has followers that aren't replicating properly, splitting
that range results in a right-hand side with followers in a similar
state. Certain workloads (restore/import/presplit) can run large numbers
of splits against a given range, and this can result in a large number
of Raft snapshots that backs up the Raft snapshot queue.

Ideally we'd never have any ranges that require a snapshot, but over
the last weeks it has become clear that this is very difficult to
achieve since the knowledge required to decide whether a snapshot
can efficiently be prevented is distributed across multiple nodes
that don't share the necessary information.

This is a bit of a nuclear option to prevent the likely last big
culprit in large numbers of Raft snapshots in #31409.

With this change, we should expect to see Raft snapshots regularly when
a split/scatter phase of an import/restore is active, but never large
volumes at once.

Release note: None

pkg/storage/replica_command.go

@@ -39,6 +39,7 @@ import (
  "github.com/cockroachdb/cockroach/pkg/util/log"
  "github.com/cockroachdb/cockroach/pkg/util/protoutil"
  "github.com/cockroachdb/cockroach/pkg/util/retry"
+ "github.com/cockroachdb/cockroach/pkg/util/timeutil"
  "github.com/pkg/errors"
  "go.etcd.io/etcd/raft"
  "go.etcd.io/etcd/raft/raftpb"
@@ -222,6 +223,71 @@ func splitSnapshotWarningStr(rangeID roachpb.RangeID, status *raft.Status) strin
  return s
 }
 
+func (r *Replica) maybeDelaySplitToAvoidSnapshot(ctx context.Context) string {
+ maxDelaySplitToAvoidSnapshotTicks := 5 + r.store.cfg.RaftPostSplitSuppressSnapshotTicks
+
+ var extra string
+
+ tPreWait := timeutil.Now()
+ tPostWait := tPreWait
+ for ticks := 0; ticks < maxDelaySplitToAvoidSnapshotTicks; ticks++ {
+ if ticks > 0 {
+ tPostWait = time.Time{}
+ }
+
+ r.mu.RLock()
+ raftStatus := r.raftStatusRLocked()
+ if raftStatus != nil {
+ updateRaftProgressFromActivity(
+ ctx, raftStatus.Progress, r.descRLocked().Replicas, r.mu.lastUpdateTimes, timeutil.Now(),
+ )
+ }
+ r.mu.RUnlock()
+
+ if raftStatus == nil {
+ // Don't delay followers artificially. This case is hit rarely
+ // enough to not matter.
+ break
+ }
+
+ done := true
+ for replicaID, pr := range raftStatus.Progress {
+ if replicaID == raftStatus.Lead {
+ // TODO(tschottdorf): remove this once we have picked up
+ // https://github.com/etcd-io/etcd/pull/10279
+ continue
+ }
+
+ if !pr.RecentActive {
+ continue
+ }
+
+ if pr.State != raft.ProgressStateReplicate {
+ if ticks == 0 {
+ extra += fmt.Sprintf("delaying split; replica r%d/%d not caught up: %+v", r.RangeID, replicaID, pr)
+ }
+ done = false
+ }
+ }
+ if done {
+ break
+ }
+ select {
+ case <-time.After(r.store.cfg.RaftTickInterval):
+ case <-ctx.Done():
+ return ""
+ }
+ }
+ if tPostWait == (time.Time{}) {
+ tPostWait = timeutil.Now()
+ }
+
+ if elapsed := tPostWait.Sub(tPreWait); elapsed != 0 {
+ extra += fmt.Sprintf("; delayed split for %s to avoid Raft snapshot", elapsed)
+ }
+ return extra
+}
+
 // adminSplitWithDescriptor divides the range into into two ranges, using
 // either args.SplitKey (if provided) or an internally computed key that aims
 // to roughly equipartition the range by size. The split is done inside of a
@@ -320,10 +386,11 @@ func (r *Replica) adminSplitWithDescriptor(
  }
  leftDesc.EndKey = splitKey
 
- extra := splitSnapshotWarningStr(r.RangeID, r.RaftStatus())
+ extra := r.maybeDelaySplitToAvoidSnapshot(ctx)
+ extra += splitSnapshotWarningStr(r.RangeID, r.RaftStatus())
 
  log.Infof(ctx, "initiating a split of this range at key %s [r%d]%s",
- splitKey, rightDesc.RangeID, extra)
+ extra, splitKey, rightDesc.RangeID)
 
  if err := r.store.DB().Txn(ctx, func(ctx context.Context, txn *client.Txn) error {
  log.Event(ctx, "split closure begins")