Merge #143122

143122: kvserver: actually subject MVCC GC to IO admission control r=tbg a=tbg

MVCC GC sends BatchRequests directly to `(*Replica).SendWithWriteBytes`. Since
admission control hooks live at a higher level `(*Node).batchInternal`, MVCC GC
duplicated the relevant hooks in `(*replicaGCer).send`. This duplication was
incomplete: the context passed to `SendWithWriteBytes` needed to be "annotated"
with the `admissionHandle` - essentially, this puts the admission metadata into
the context, so that it can be picked up at Raft proposal time to encode the
admission control policy into the replicated log entry.

Because this annotation was missing, the log entries were encoded as not being
subject to admission control at all.

I diagnosed this all via hacky printf debugging on `release-23.2`. Clearly we
have no test coverage, or this wouldn't have been undetected for so long.

Touches https://github.com/cockroachlabs/support/issues/3213.

Epic: none
Release note (bug fix): MVCC garbage collection is now fully subject to IO
admission control. Previously, it was possible for MVCC GC to cause store
overload (such as LSM inversion) when a large amounts of data would become
eligible for garbage collection.

Co-authored-by: Tobias Grieger <[email protected]>

Author: craig[bot]

pkg/kv/kvserver/mvcc_gc_queue.go

@@ -123,6 +123,16 @@ var EnqueueInMvccGCQueueOnSpanConfigUpdateEnabled = settings.RegisterBoolSetting
 	false,
 )
 
+// See https://github.com/cockroachdb/cockroach/pull/143122.
+var mvccGCQueueFullyEnableAC = settings.RegisterBoolSetting(
+	settings.SystemOnly,
+	"kv.mvcc_gc.queue_kv_admission_control.enabled",
+	"when true, MVCC GC queue operations are subject to store admission control. If set to false, "+
+		"since store admission control will be disabled, replication flow control will also be effectively disabled. "+
+		"This setting does not affect CPU admission control.",
+	true,
+)
+
 func largeAbortSpan(ms enginepb.MVCCStats) bool {
 	// Checks if the size of the abort span exceeds the given threshold.
 	// The abort span is not supposed to become that large, but it does
@@ -609,6 +619,9 @@ func (r *replicaGCer) send(ctx context.Context, req kvpb.GCRequest) error {
 		if err != nil {
 			return err
 		}
+		if mvccGCQueueFullyEnableAC.Get(&r.repl.ClusterSettings().SV) {
+			ctx = admissionHandle.AnnotateCtx(ctx)
+		}
 	}
 	_, writeBytes, pErr := r.repl.SendWithWriteBytes(ctx, ba)
 	defer writeBytes.Release()