Merge #73023

73023: kvserver: don't allow manually enqueueing uninitialized replicas r=aliher1911,tbg a=erikgrinaker

Normally, `baseQueue.maybeAdd()` prevents uninitialized replicas from
being enqueued in e.g. the replica GC queue. However, this was not
checked when manually enqueueing ranges via `Store.ManuallyEnqueue()`.
This could e.g. cause the replica GC queue to crash the local node when
it tried to remove an uninitialized replica via `Store.RemoveReplica()`.

This patch adds a check to `Store.ManuallyEnqueue()` that returns an
error if the replica is uninitialized, to avoid crashing the node, and
to indicate to the user why the range could not be enqueued.

Resolves #62709.

Release note (bug fix): Manually enqueueing ranges via the DB Console
will no longer crash nodes that contain an uninitialized replica for the
enqueued range.

Co-authored-by: Erik Grinaker <[email protected]>

pkg/kv/kvserver/store.go

@@ -3170,6 +3170,13 @@ func (s *Store) ManuallyEnqueue(
 ) (recording tracing.Recording, processError error, enqueueError error) {
 	ctx = repl.AnnotateCtx(ctx)
 
+	// Do not enqueue uninitialized replicas. The baseQueue ignores these during
+	// normal queue scheduling, but we error here to signal to the user that the
+	// operation was unsuccessful.
+	if !repl.IsInitialized() {
+		return nil, nil, errors.Errorf("not enqueueing uninitialized replica %s", repl)
+	}
+
 	var queue queueImpl
 	var needsLease bool
 	for _, replicaQueue := range s.scanner.queues {

pkg/kv/kvserver/store_test.go

@@ -2985,6 +2985,28 @@ func TestSnapshotRateLimit(t *testing.T) {
 	}
 }
 
+// TestManuallyEnqueueUninitializedReplica makes sure that uninitialized
+// replicas cannot be enqueued.
+func TestManuallyEnqueueUninitializedReplica(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	ctx := context.Background()
+	stopper := stop.NewStopper()
+	defer stopper.Stop(ctx)
+	tc := testContext{}
+	tc.Start(t, stopper)
+
+	repl, _, _ := tc.store.getOrCreateReplica(ctx, 42, 7, &roachpb.ReplicaDescriptor{
+		NodeID:    tc.store.NodeID(),
+		StoreID:   tc.store.StoreID(),
+		ReplicaID: 7,
+	})
+	_, _, err := tc.store.ManuallyEnqueue(ctx, "replicaGC", repl, true)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "not enqueueing uninitialized replica")
+}
+
 func BenchmarkStoreGetReplica(b *testing.B) {
 	stopper := stop.NewStopper()
 	defer stopper.Stop(context.Background())