rpc: move system ranges to system RPC class

Move all the ranges between /Min and /System/tsd to
use the default RPC class. This will allow for
isolation from network congestion for all system
ranges, which crucial for the stability of the system.

Fixes: cockroachdb#111239

Release note: None

pkg/kv/kvclient/kvcoord/dist_sender_test.go

@@ -4289,35 +4289,47 @@ func TestConnectionClass(t *testing.T) {
 	defer stopper.Stop(ctx)
 	// Create a mock range descriptor DB that can resolve made up meta1, node
 	// liveness and user ranges.
+
+	replicas := []roachpb.ReplicaDescriptor{
+		{NodeID: 1, StoreID: 1},
+	}
 	rDB := MockRangeDescriptorDB(func(key roachpb.RKey, _ bool) (
 		[]roachpb.RangeDescriptor, []roachpb.RangeDescriptor, error,
 	) {
 		if keys.RangeMetaKey(key).Equal(roachpb.RKeyMin) {
 			return []roachpb.RangeDescriptor{{
-				RangeID:  1,
-				StartKey: roachpb.RKeyMin,
-				EndKey:   roachpb.RKey(keys.NodeLivenessPrefix),
-				InternalReplicas: []roachpb.ReplicaDescriptor{
-					{NodeID: 1, StoreID: 1},
-				},
+				RangeID:          1,
+				StartKey:         roachpb.RKeyMin,
+				EndKey:           roachpb.RKey(keys.NodeLivenessPrefix),
+				InternalReplicas: replicas,
 			}}, nil, nil
 		} else if bytes.HasPrefix(key, keys.NodeLivenessPrefix) {
 			return []roachpb.RangeDescriptor{{
-				RangeID:  2,
-				StartKey: roachpb.RKey(keys.NodeLivenessPrefix),
-				EndKey:   roachpb.RKey(keys.NodeLivenessKeyMax),
-				InternalReplicas: []roachpb.ReplicaDescriptor{
-					{NodeID: 1, StoreID: 1},
-				},
+				RangeID:          2,
+				StartKey:         roachpb.RKey(keys.NodeLivenessPrefix),
+				EndKey:           roachpb.RKey(keys.NodeLivenessKeyMax),
+				InternalReplicas: replicas,
+			}}, nil, nil
+		} else if bytes.Compare(key.AsRawKey(), keys.NodeLivenessKeyMax) >= 0 && bytes.Compare(key.AsRawKey(), keys.TimeseriesPrefix) < 0 {
+			return []roachpb.RangeDescriptor{{
+				RangeID:          3,
+				StartKey:         roachpb.RKey(keys.NodeLivenessKeyMax),
+				EndKey:           roachpb.RKey(keys.TimeseriesPrefix),
+				InternalReplicas: replicas,
+			}}, nil, nil
+		} else if bytes.HasPrefix(key, keys.TimeseriesPrefix) {
+			return []roachpb.RangeDescriptor{{
+				RangeID:          4,
+				StartKey:         roachpb.RKey(keys.TimeseriesPrefix),
+				EndKey:           roachpb.RKey(keys.TimeseriesKeyMax),
+				InternalReplicas: replicas,
 			}}, nil, nil
 		}
 		return []roachpb.RangeDescriptor{{
-			RangeID:  3,
-			StartKey: roachpb.RKey(keys.NodeLivenessKeyMax),
-			EndKey:   roachpb.RKeyMax,
-			InternalReplicas: []roachpb.ReplicaDescriptor{
-				{NodeID: 1, StoreID: 1},
-			},
+			RangeID:          5,
+			StartKey:         roachpb.RKey(keys.TimeseriesKeyMax),
+			EndKey:           roachpb.RKeyMax,
+			InternalReplicas: replicas,
 		}}, nil, nil
 	})
 
@@ -4349,28 +4361,32 @@ func TestConnectionClass(t *testing.T) {
 	}
 	ds := NewDistSender(cfg)
 
-	// Check the three important cases to ensure they are sent with the correct
-	// ConnectionClass.
-	for _, key := range []roachpb.Key{
-		keys.Meta1Prefix,
-		keys.NodeLivenessKey(1),
-		keys.SystemSQLCodec.TablePrefix(1234), // A non-system table
+	for _, pair := range []struct {
+		key       roachpb.Key
+		wantClass rpc.ConnectionClass
+	}{
+		{key: keys.Meta1Prefix, wantClass: rpc.SystemClass},
+		{key: keys.NodeLivenessKey(1), wantClass: rpc.SystemClass},
+		{key: keys.StatusNodePrefix, wantClass: rpc.SystemClass},
+		{key: keys.NodeStatusKey(15), wantClass: rpc.SystemClass},
+		{key: keys.NodeIDGenerator, wantClass: rpc.SystemClass},
+		{key: keys.TimeseriesPrefix, wantClass: rpc.DefaultClass},
+		{key: keys.SystemSpanConfigPrefix, wantClass: rpc.DefaultClass},
+		{key: keys.SystemSQLCodec.TablePrefix(1234), wantClass: rpc.DefaultClass},
 	} {
-		t.Run(key.String(), func(t *testing.T) {
+		t.Run(pair.key.String(), func(t *testing.T) {
 			ba := &kvpb.BatchRequest{}
 			ba.Add(&kvpb.GetRequest{
 				RequestHeader: kvpb.RequestHeader{
-					Key: key,
+					Key: pair.key,
 				},
 			})
 			_, pErr := ds.Send(context.Background(), ba)
 			require.Nil(t, pErr)
 
 			// Verify that the request carries the class we expect it to for its span.
-			span, err := keys.Range(ba.Requests)
-			require.NoError(t, err)
-			require.Equalf(t, rpc.ConnectionClassForKey(span.Key, rpc.DefaultClass), class,
-				"unexpected class for span key %v", span.Key)
+			require.Equalf(t, pair.wantClass, class,
+				"unexpected class for span key %v", pair.key)
 		})
 	}
 }

pkg/rpc/connection_class.go

@@ -11,8 +11,6 @@
 package rpc
 
 import (
-	"bytes"
-
 	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/rpc/rpcpb"
@@ -55,29 +53,19 @@ const (
 	NumConnectionClasses = int(rpcpb.ConnectionClass_NEXT)
 )
 
-var systemClassKeyPrefixes = []roachpb.RKey{
-	roachpb.RKey(keys.Meta1Prefix),
-	roachpb.RKey(keys.NodeLivenessPrefix),
-}
-
-// isSystemKey returns true if the given key belongs to a range eligible for
-// SystemClass connection.
+// systemClassSpan is the key span for all ranges considered to be a critical
+// system range. Generally, not all system ranges are critical. For example, the
+// timeseries ranges are not, because they can be busy and disrupt other
+// system traffic. We try to make SystemClass responsive by keeping it small.
 //
-// Generally, not all system ranges are eligible. For example, the timeseries
-// ranges are not, because they can be busy and disrupt other system traffic. We
-// try to make SystemClass responsive by keeping it small.
-func isSystemKey(key roachpb.RKey) bool {
-	// An empty RKey addresses range 1 and warrants SystemClass.
-	if len(key) == 0 {
-		return true
-	}
-	for _, prefix := range systemClassKeyPrefixes {
-		if bytes.HasPrefix(key, prefix) {
-			return true
-		}
-	}
-	return false
-}
+// All the ranges that include keys between /Meta1 and/ System/tsd are considered
+// part of the system class. We keep things simple by checking if the start key
+// of a range is within the system span to determine if we should use the
+// SystemClass. This eliminates most of the coupling between how system ranges
+// are structured (i.e. #NoSplits and # staticSplits lists) and how they
+// split/merge except at the boundaries. Specifically it's safe to use
+// keys.TimeseriesPrefix because nothing >= to this key will merge into this span.
+var systemClassSpan = roachpb.Span{Key: keys.Meta1Prefix, EndKey: keys.TimeseriesPrefix}
 
 // ConnectionClassForKey determines the ConnectionClass which should be used for
 // traffic addressed to the range starting at the given key. Returns SystemClass
@@ -87,7 +75,10 @@ func isSystemKey(key roachpb.RKey) bool {
 // This also takes the `COCKROACH_RPC_USE_DEFAULT_CONNECTION_CLASS` env variable
 // into account, and returns DefaultClass instead of `def` if it is set.
 func ConnectionClassForKey(key roachpb.RKey, def ConnectionClass) ConnectionClass {
-	if isSystemKey(key) {
+	// We have to check for special condition of 0 length key, to catch Range 1.
+	// Once https://github.com/cockroachdb/cockroach/issues/95055 is fixed, we can
+	// remove this check.
+	if len(key) == 0 || systemClassSpan.ContainsKey(key.AsRawKey()) {
 		return SystemClass
 	}
 	return ConnectionClassOrDefault(def)