diff --git a/pkg/kv/kvclient/kvcoord/dist_sender_test.go b/pkg/kv/kvclient/kvcoord/dist_sender_test.go index 18678fdc999f..68b5fa67f476 100644 --- a/pkg/kv/kvclient/kvcoord/dist_sender_test.go +++ b/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) }) } } diff --git a/pkg/rpc/connection_class.go b/pkg/rpc/connection_class.go index c68497a5a557..ca2515a43c65 100644 --- a/pkg/rpc/connection_class.go +++ b/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)