From 64d30db14633f63478812c7ecb8c7bde3aa5cc70 Mon Sep 17 00:00:00 2001 From: Brian Dillmann Date: Tue, 22 Oct 2024 12:03:37 -0400 Subject: [PATCH 1/4] sql: using catalog reader, allow descriptor scanning by span MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Historically, to estimate which tables and indexes existed within the span of a range, we used the Collection's `GetAllDescriptors` function and returned the values which fell in between the start and end keys of the range's span. This approach had the benefit of being precise, but the drawback of being computationally expensive - since the system can have hundreds of nodes and theoretically millions of tables, using `GetAllDescriptors` begins to become a bottleneck. This approach was modified so that instead of pulling all descriptors for the system when computing range contents, the system instead used the start key of the range to identify the exact table + index at that point within the keyspace ([change](https://github.com/cockroachdb/cockroach/pull/77277/files)). This works if each table, index has their own range, but quickly breaks down if tables share a range. Consider the following layout of data: ``` T1=Table 1 T2=Table 2 R1=Range1 └─────T1─────┴─────T2────┴─────T3─────┘ └────────┴─────────R1───────┴─────────┘ ``` Since the start key of the range falls with Table 1, the system associates the range with only Table 1, despite it containing Tables 2 and 3. Using this information, it becomes necessary to identify a set of descriptors within a certain span. This PR introduces the `ScanDescriptorsInSpans` function which does just that, allows the user to specify a set of spans whose descriptors are important and then return a catalog including those descriptors. It does this by translating the span keys into descriptor span keys and scanning them from the descriptors table. For example given a span `[/Table/Users/PKEY/1, /Table/Users/SECONDARY/chicago]` where the ID for the Users table is `5`, it will generate a descriptor span `[/Table/Descriptors/PKEY/5, /Table/Descriptors/PKEY/6]`. This descriptor too comes with its drawbacks in that within the descriptor space, keys are scoped by table, and not necessarily indexes. That means in a following PR, the status server will be responsible for taking these descriptors, which include all indexes in the tables pulled, and filtering it down to only the indexes which appear in the specified range. The bulk of the changeset is in updating the datadriven tests to test this behavior, the primary area of focus for review should be the `pkg/sql/catalog/internal/catkv/catalog_reader.go` file (~75 LOC). Epic: CRDB-43151 Fixes: #130997 Release note: None --- pkg/sql/catalog/descs/collection.go | 7 + pkg/sql/catalog/internal/catkv/BUILD.bazel | 2 + .../catalog/internal/catkv/catalog_reader.go | 73 +++++++ .../internal/catkv/catalog_reader_cached.go | 10 + .../internal/catkv/catalog_reader_test.go | 72 +++++++ .../internal/catkv/testdata/testdata_app | 203 ++++++++++++++++++ .../internal/catkv/testdata/testdata_system | 203 ++++++++++++++++++ 7 files changed, 570 insertions(+) diff --git a/pkg/sql/catalog/descs/collection.go b/pkg/sql/catalog/descs/collection.go index 66963635af2c..898db4063400 100644 --- a/pkg/sql/catalog/descs/collection.go +++ b/pkg/sql/catalog/descs/collection.go @@ -743,6 +743,13 @@ func (tc *Collection) GetAll(ctx context.Context, txn *kv.Txn) (nstree.Catalog, return ret.Catalog, nil } +// GetDescriptorsInSpans returns all descriptors within a given span. +func (tc *Collection) GetDescriptorsInSpans( + ctx context.Context, txn *kv.Txn, spans []roachpb.Span, +) (nstree.Catalog, error) { + return tc.cr.ScanDescriptorsInSpans(ctx, txn, spans) +} + // GetAllComments gets all comments for all descriptors in the given database. // This method never returns the underlying catalog, since it will be incomplete and only // contain comments. diff --git a/pkg/sql/catalog/internal/catkv/BUILD.bazel b/pkg/sql/catalog/internal/catkv/BUILD.bazel index c1c1ec3f74ab..f730b814c152 100644 --- a/pkg/sql/catalog/internal/catkv/BUILD.bazel +++ b/pkg/sql/catalog/internal/catkv/BUILD.bazel @@ -55,6 +55,7 @@ go_test( ":catkv", "//pkg/base", "//pkg/kv", + "//pkg/roachpb", "//pkg/security/securityassets", "//pkg/security/securitytest", "//pkg/server", @@ -67,6 +68,7 @@ go_test( "//pkg/testutils/datapathutils", "//pkg/testutils/serverutils", "//pkg/testutils/sqlutils", + "//pkg/util/encoding", "//pkg/util/leaktest", "//pkg/util/log", "//pkg/util/randutil", diff --git a/pkg/sql/catalog/internal/catkv/catalog_reader.go b/pkg/sql/catalog/internal/catkv/catalog_reader.go index c4afb3cd9a5b..ed316ce5b823 100644 --- a/pkg/sql/catalog/internal/catkv/catalog_reader.go +++ b/pkg/sql/catalog/internal/catkv/catalog_reader.go @@ -49,6 +49,9 @@ type CatalogReader interface { // ScanAll scans the entirety of the descriptor and namespace tables. ScanAll(ctx context.Context, txn *kv.Txn) (nstree.Catalog, error) + // ScanDescriptorsInSpans scans the descriptors specified in a given span. + ScanDescriptorsInSpans(ctx context.Context, txn *kv.Txn, span []roachpb.Span) (nstree.Catalog, error) + // ScanAllComments scans the entirety of the comments table as well as the namespace entries for the given database. // If the dbContext is nil, we scan the database-level namespace entries. ScanAllComments(ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor) (nstree.Catalog, error) @@ -158,6 +161,76 @@ func (cr catalogReader) ScanAll(ctx context.Context, txn *kv.Txn) (nstree.Catalo return mc.Catalog, nil } +// getDescriptorIDFromExclusiveKey translates an exclusive upper bound roach key +// into an upper bound descriptor ID. It does this by turning the key into a +// descriptor ID, and then moving it upwards if and only if it is not the prefix +// of the current index / table span. +func getDescriptorIDFromExclusiveKey(codec keys.SQLCodec, key roachpb.Key) (uint32, error) { + keyWithoutTable, endID, err := codec.DecodeTablePrefix(key) + if err != nil { + return 0, err + } + if len(keyWithoutTable) == 0 { + return endID, nil + } + + keyWithoutIndex, _, indexId, err := codec.DecodeIndexPrefix(key) + if err != nil { + return 0, err + } + // if there's remaining bytes or the index isn't the primary, increment + // the end so that the descriptor under the key is included. + if len(keyWithoutIndex) != 0 || indexId > 1 { + endID++ + } + return endID, nil +} + +// getDescriptorSpanFromSpan returns a start and end descriptor ID from a given span +func getDescriptorSpanFromSpan(codec keys.SQLCodec, span roachpb.Span) (roachpb.Span, error) { + _, startID, err := codec.DecodeTablePrefix(span.Key) + if err != nil { + return roachpb.Span{}, err + } + endID, err := getDescriptorIDFromExclusiveKey(codec, span.EndKey) + if err != nil { + return roachpb.Span{}, err + } + + return roachpb.Span{ + Key: catalogkeys.MakeDescMetadataKey(codec, descpb.ID(startID)), + EndKey: catalogkeys.MakeDescMetadataKey(codec, descpb.ID(endID)), + }, nil +} + +// ScanDescriptorsInSpans is part of the CatalogReader interface. +func (cr catalogReader) ScanDescriptorsInSpans( + ctx context.Context, txn *kv.Txn, spans []roachpb.Span, +) (nstree.Catalog, error) { + var mc nstree.MutableCatalog + + descSpans := make([]roachpb.Span, len(spans)) + for i, span := range spans { + descSpan, err := getDescriptorSpanFromSpan(cr.Codec(), span) + if err != nil { + return mc.Catalog, err + } + descSpans[i] = descSpan + } + + cq := catalogQuery{codec: cr.codec} + err := cq.query(ctx, txn, &mc, func(codec keys.SQLCodec, b *kv.Batch) { + for _, descSpan := range descSpans { + scanRange(ctx, b, descSpan.Key, descSpan.EndKey) + } + }) + if err != nil { + return mc.Catalog, err + } + + return mc.Catalog, nil +} + // ScanAllComments is part of the CatalogReader interface. func (cr catalogReader) ScanAllComments( ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor, diff --git a/pkg/sql/catalog/internal/catkv/catalog_reader_cached.go b/pkg/sql/catalog/internal/catkv/catalog_reader_cached.go index 521e5dc6b46d..213c562ae72b 100644 --- a/pkg/sql/catalog/internal/catkv/catalog_reader_cached.go +++ b/pkg/sql/catalog/internal/catkv/catalog_reader_cached.go @@ -11,6 +11,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/clusterversion" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/nstree" @@ -220,6 +221,15 @@ func (c *cachedCatalogReader) ScanAll(ctx context.Context, txn *kv.Txn) (nstree. return read, nil } +// ScanDescriptorsInSpans is part of the CatalogReader interface. +func (c *cachedCatalogReader) ScanDescriptorsInSpans( + ctx context.Context, txn *kv.Txn, spans []roachpb.Span, +) (nstree.Catalog, error) { + // TODO (brian.dillmann@): explore caching these calls. + // https://github.com/cockroachdb/cockroach/issues/134666 + return c.cr.ScanDescriptorsInSpans(ctx, txn, spans) +} + // ScanNamespaceForDatabases is part of the CatalogReader interface. func (c *cachedCatalogReader) ScanNamespaceForDatabases( ctx context.Context, txn *kv.Txn, diff --git a/pkg/sql/catalog/internal/catkv/catalog_reader_test.go b/pkg/sql/catalog/internal/catkv/catalog_reader_test.go index 52730414cea1..42ecd71c9a63 100644 --- a/pkg/sql/catalog/internal/catkv/catalog_reader_test.go +++ b/pkg/sql/catalog/internal/catkv/catalog_reader_test.go @@ -16,6 +16,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/base" "github.com/cockroachdb/cockroach/pkg/kv" + "github.com/cockroachdb/cockroach/pkg/roachpb" "github.com/cockroachdb/cockroach/pkg/sql" "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkeys" @@ -26,6 +27,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils/datapathutils" "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" + "github.com/cockroachdb/cockroach/pkg/util/encoding" "github.com/cockroachdb/cockroach/pkg/util/leaktest" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/tracing" @@ -203,6 +205,24 @@ func TestDataDriven(t *testing.T) { return cr.GetByNames(ctx, txn, nis) } return h.doCatalogQuery(ctx, q) + case "scan_descriptors_in_span": + { + start := h.parseKeyFromArgKey("start") + end := h.parseKeyFromArgKey("end") + q := func(ctx context.Context, txn *kv.Txn, cr catkv.CatalogReader) (nstree.Catalog, error) { + return cr.ScanDescriptorsInSpans(ctx, txn, []roachpb.Span{{Key: start, EndKey: end}}) + } + return h.doCatalogQuery(ctx, q) + } + case "scan_descriptors_in_multiple_spans": + { + first := h.parseSpanFromArgKey("first") + second := h.parseSpanFromArgKey("second") + q := func(ctx context.Context, txn *kv.Txn, cr catkv.CatalogReader) (nstree.Catalog, error) { + return cr.ScanDescriptorsInSpans(ctx, txn, []roachpb.Span{first, second}) + } + return h.doCatalogQuery(ctx, q) + } } return fmt.Sprintf("%s: unknown command: %s", d.Pos, d.Cmd) }) @@ -217,6 +237,58 @@ type testHelper struct { ucr, ccr catkv.CatalogReader } +func (h testHelper) parseSpanFromArgKey(argkey string) roachpb.Span { + arg, exists := h.d.Arg(argkey) + if !exists { + h.t.Fatalf("scan_descriptors_in_span requires '%s' arg", argkey) + } + start, end := arg.TwoVals(h.t) + return roachpb.Span{ + Key: h.parseKeyFromArgStr(start), + EndKey: h.parseKeyFromArgStr(end), + } +} + +func (h testHelper) parseKeyFromArgKey(argkey string) roachpb.Key { + arg, exists := h.d.Arg(argkey) + if !exists { + h.t.Fatalf("scan_descriptors_in_span requires '%s' arg", argkey) + } + return h.parseKeyFromArgStr(arg.SingleVal(h.t)) +} + +func (h testHelper) parseKeyFromArgStr(argstr string) roachpb.Key { + parts := strings.Split(argstr, "/") + if len(parts) == 0 { + h.t.Fatal("cannot parse key without at least one key part") + } else if len(parts) > 4 { + h.t.Fatal("key argument has too many parts") + } + + tableId, err := strconv.Atoi(parts[0]) + require.NoError(h.t, err) + if len(parts) == 1 { + return h.execCfg.Codec.TablePrefix(uint32(tableId)) + } + + indexId, err := strconv.Atoi(parts[1]) + require.NoError(h.t, err) + + key := h.execCfg.Codec.IndexPrefix(uint32(tableId), uint32(indexId)) + if len(parts) == 3 && parts[2] != "" { + // only supports integer and string key values + if encoding.PeekType([]byte(parts[2])) == encoding.Int { + pkey, err := strconv.Atoi(parts[1]) + require.NoError(h.t, err) + return encoding.EncodeVarintAscending(key, int64(pkey)) + } else { + return encoding.EncodeStringAscending(key, parts[2]) + } + } + + return key +} + func (h testHelper) argDesc( ctx context.Context, idArgName string, expectedType catalog.DescriptorType, ) catalog.Descriptor { diff --git a/pkg/sql/catalog/internal/catkv/testdata/testdata_app b/pkg/sql/catalog/internal/catkv/testdata/testdata_app index fa751bade8f8..dd6ac6bbb6b0 100644 --- a/pkg/sql/catalog/internal/catkv/testdata/testdata_app +++ b/pkg/sql/catalog/internal/catkv/testdata/testdata_app @@ -16,6 +16,18 @@ COMMENT ON TABLE kv IS 'this is a table'; COMMENT ON INDEX mv@idx IS 'this is an index'; COMMENT ON CONSTRAINT ck ON kv IS 'this is a check constraint'; COMMENT ON CONSTRAINT kv_pkey ON kv IS 'this is a primary key constraint'; + +-- below queries are for scan_descriptors_in_span tests +CREATE TABLE scan_test_1(main SERIAL PRIMARY KEY, alternate VARCHAR UNIQUE); +CREATE TABLE scan_test_2(main INT PRIMARY KEY, alternate VARCHAR UNIQUE); + +INSERT INTO scan_test_1(alternate) VALUES ('a'); +INSERT INTO scan_test_1(alternate) VALUES ('c'); +INSERT INTO scan_test_1(alternate) VALUES ('f'); + +INSERT INTO scan_test_2(main, alternate) VALUES (1, 'b'); +INSERT INTO scan_test_2(main, alternate) VALUES (4, 'c'); +INSERT INTO scan_test_2(main, alternate) VALUES (9, 'd'); ---- scan_namespace_for_databases @@ -70,6 +82,10 @@ catalog: namespace: (100, 101, "kv") "109": namespace: (100, 101, "mv") + "110": + namespace: (100, 101, "scan_test_1") + "111": + namespace: (100, 101, "scan_test_2") trace: - Scan /NamespaceTable/30/1/100 @@ -421,6 +437,12 @@ catalog: index_2: this is an index descriptor: relation namespace: (100, 101, "mv") + "110": + descriptor: relation + namespace: (100, 101, "scan_test_1") + "111": + descriptor: relation + namespace: (100, 101, "scan_test_2") trace: - Scan /Table/3/1 - Scan /NamespaceTable/30/1 @@ -512,6 +534,10 @@ catalog: comments: index_2: this is an index namespace: (100, 101, "mv") + "110": + namespace: (100, 101, "scan_test_1") + "111": + namespace: (100, 101, "scan_test_2") trace: - Scan /NamespaceTable/30/1/100 - Scan /Table/24/1 @@ -551,3 +577,180 @@ catalog: trace: - Scan /NamespaceTable/30/1/0 - Scan /Table/24/1 + +# The below tests test the many circumstances for scanning a set +# of descriptors within a span. +# +# For this test, there are two relevant tables, 'scan_test_1' and 'scan_test_2' +# with two indexes each on columns main and secondary, denoted +# by the below notation: +# T1I1 (scan_test_1, main), T1I2, T2I1, T2I2 +# +# The boundaries will be marked by keys on either side of it /. +# +# Indexes = └────T1I1────┴────T1I2───┴────T2I1────┴────T2I2───┘ +# Keys = min/1 3/a f/1 9/b d/max +# +# Args 'start' and 'end' take the format "/(/?)" + +# start key after end key should panic +# disabled because the panic seems to appear in a different goroutine +# scan_descriptors_in_span start=111/1 end=110/1 panics=true +# ---- + +# # same start and end key should return first descriptor +# disabled because it panics if the same key is passed +# scan_descriptors_in_span start=110/1 end=110/1 +# ---- + +# test with only table prefix +scan_descriptors_in_span start=110 end=111 +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + +# start and end are one key away from each other +scan_descriptors_in_span start=110/1/1 end=110/1/2 +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + +# scan with the start at the prefix, the end in the table span +scan_descriptors_in_span start=110/1 end=110/1/1 +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + +# end after the first index +scan_descriptors_in_span start=110/1 end=110/2 +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# end in the second index +scan_descriptors_in_span start=110/1 end=110/2/a +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# end on a value which doesn't exist +scan_descriptors_in_span start=110/1 end=110/2/b +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# end on the last value in the second index +scan_descriptors_in_span start=110/1 end=110/2/f +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# start on the last value in the second index +scan_descriptors_in_span start=110/1 end=110/2/f +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# end directly on first index is exclusive +scan_descriptors_in_span start=110/1 end=111/1 +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# end on the first key of the second table is inclusive +scan_descriptors_in_span start=110/1 end=111/1/0 +---- +catalog: + "110": + descriptor: relation + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/112/2/1 + +# end on an absurd key +scan_descriptors_in_span start=110/2/f end=9000/1 +---- +catalog: + "110": + descriptor: relation + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/9000/2/1 + +# start in the middle of the first table +scan_descriptors_in_span start=110/1/1 end=112/1 +---- +catalog: + "110": + descriptor: relation + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/112/2/1 + +# start on the last key of the first table +scan_descriptors_in_span start=110/2/f end=112/1 +---- +catalog: + "110": + descriptor: relation + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/112/2/1 + +# start on the first key of the second table +scan_descriptors_in_span start=111/1 end=112/1 +---- +catalog: + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/111/2/1 /Table/3/1/112/2/1 + +# verify that multiple span scanning works +scan_descriptors_in_multiple_spans first=(110/1,111/1) second=(111/1,112/1) +---- +catalog: + "110": + descriptor: relation + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 +- Scan Range /Table/3/1/111/2/1 /Table/3/1/112/2/1 diff --git a/pkg/sql/catalog/internal/catkv/testdata/testdata_system b/pkg/sql/catalog/internal/catkv/testdata/testdata_system index 829b2dd83378..58b78e92fdb2 100644 --- a/pkg/sql/catalog/internal/catkv/testdata/testdata_system +++ b/pkg/sql/catalog/internal/catkv/testdata/testdata_system @@ -16,6 +16,18 @@ COMMENT ON TABLE kv IS 'this is a table'; COMMENT ON INDEX mv@idx IS 'this is an index'; COMMENT ON CONSTRAINT ck ON kv IS 'this is a check constraint'; COMMENT ON CONSTRAINT kv_pkey ON kv IS 'this is a primary key constraint'; + +-- below queries are for scan_descriptors_in_span tests +CREATE TABLE scan_test_1(main SERIAL PRIMARY KEY, alternate VARCHAR UNIQUE); +CREATE TABLE scan_test_2(main INT PRIMARY KEY, alternate VARCHAR UNIQUE); + +INSERT INTO scan_test_1(alternate) VALUES ('a'); +INSERT INTO scan_test_1(alternate) VALUES ('c'); +INSERT INTO scan_test_1(alternate) VALUES ('f'); + +INSERT INTO scan_test_2(main, alternate) VALUES (1, 'b'); +INSERT INTO scan_test_2(main, alternate) VALUES (4, 'c'); +INSERT INTO scan_test_2(main, alternate) VALUES (9, 'd'); ---- scan_namespace_for_databases @@ -70,6 +82,10 @@ catalog: namespace: (100, 101, "kv") "109": namespace: (100, 101, "mv") + "110": + namespace: (100, 101, "scan_test_1") + "111": + namespace: (100, 101, "scan_test_2") trace: - Scan /NamespaceTable/30/1/100 @@ -439,6 +455,12 @@ catalog: index_2: this is an index descriptor: relation namespace: (100, 101, "mv") + "110": + descriptor: relation + namespace: (100, 101, "scan_test_1") + "111": + descriptor: relation + namespace: (100, 101, "scan_test_2") trace: - Scan /Table/3/1 - Scan /NamespaceTable/30/1 @@ -530,6 +552,10 @@ catalog: comments: index_2: this is an index namespace: (100, 101, "mv") + "110": + namespace: (100, 101, "scan_test_1") + "111": + namespace: (100, 101, "scan_test_2") trace: - Scan /NamespaceTable/30/1/100 - Scan /Table/24/1 @@ -569,3 +595,180 @@ catalog: trace: - Scan /NamespaceTable/30/1/0 - Scan /Table/24/1 + +# The below tests test the many circumstances for scanning a set +# of descriptors within a span. +# +# For this test, there are two relevant tables, 'scan_test_1' and 'scan_test_2' +# with two indexes each on columns main and secondary, denoted +# by the below notation: +# T1I1 (scan_test_1, main), T1I2, T2I1, T2I2 +# +# The boundaries will be marked by keys on either side of it /. +# +# Indexes = └────T1I1────┴────T1I2───┴────T2I1────┴────T2I2───┘ +# Keys = min/1 3/a f/1 9/b d/max +# +# Args 'start' and 'end' take the format "/(/?)" + +# start key after end key should panic +# disabled because the panic seems to appear in a different goroutine +# scan_descriptors_in_span start=111/1 end=110/1 panics=true +# ---- + +# # same start and end key should return first descriptor +# disabled because it panics if the same key is passed +# scan_descriptors_in_span start=110/1 end=110/1 +# ---- + +# test with only table prefix +scan_descriptors_in_span start=110 end=111 +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + +# start and end are one key away from each other +scan_descriptors_in_span start=110/1/1 end=110/1/2 +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + +# scan with the start at the prefix, the end in the table span +scan_descriptors_in_span start=110/1 end=110/1/1 +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + +# end after the first index +scan_descriptors_in_span start=110/1 end=110/2 +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# end in the second index +scan_descriptors_in_span start=110/1 end=110/2/a +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# end on a value which doesn't exist +scan_descriptors_in_span start=110/1 end=110/2/b +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# end on the last value in the second index +scan_descriptors_in_span start=110/1 end=110/2/f +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# start on the last value in the second index +scan_descriptors_in_span start=110/1 end=110/2/f +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# end directly on first index is exclusive +scan_descriptors_in_span start=110/1 end=111/1 +---- +catalog: + "110": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 + + +# end on the first key of the second table is inclusive +scan_descriptors_in_span start=110/1 end=111/1/0 +---- +catalog: + "110": + descriptor: relation + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/112/2/1 + +# end on an absurd key +scan_descriptors_in_span start=110/2/f end=9000/1 +---- +catalog: + "110": + descriptor: relation + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/9000/2/1 + +# start in the middle of the first table +scan_descriptors_in_span start=110/1/1 end=112/1 +---- +catalog: + "110": + descriptor: relation + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/112/2/1 + +# start on the last key of the first table +scan_descriptors_in_span start=110/2/f end=112/1 +---- +catalog: + "110": + descriptor: relation + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/112/2/1 + +# start on the first key of the second table +scan_descriptors_in_span start=111/1 end=112/1 +---- +catalog: + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/111/2/1 /Table/3/1/112/2/1 + +# verify that multiple span scanning works +scan_descriptors_in_multiple_spans first=(110/1,111/1) second=(111/1,112/1) +---- +catalog: + "110": + descriptor: relation + "111": + descriptor: relation +trace: +- Scan Range /Table/3/1/110/2/1 /Table/3/1/111/2/1 +- Scan Range /Table/3/1/111/2/1 /Table/3/1/112/2/1 From 3f947052bfa67d77fa85db4f06c44b782d138b76 Mon Sep 17 00:00:00 2001 From: Brian Dillmann Date: Wed, 30 Oct 2024 09:58:04 -0400 Subject: [PATCH 2/4] apiutil, roachpb: create utilities to map descriptors to ranges Previously each range correlated to a single table, or even a single index in a database, so all that was required to identify which tables, indexes were in the range were to look at the start key of the range and map it accordingly. With range coalescing however, it's possible for one, or many, tables, indexes and the like to reside within the same range. To properly identify the contents of a range, this PR adds the following utilities: 1. A utility function which turns a range into a span, and clamps it to its tenant's table space. 2. A utility function which takes the above spans and uses the catalog and new descriptor by span utility to turn those spans into a set of table descriptors ordered by id. 3. A utility function which transforms those table descriptors into a set of (database, table, index) names which deduplicate and identify each index uniquely. 4. A utility function, which merges the ranges and indexes into a map keyed by RangeID whose values are the above index names. 5. A primary entrypoint for consumers from which a set of ranges can be passed in and a mapping from those ranges to indexes can be returned. A variety of caveats come with this approach. It attempts to scan the desciptors all at once, but it still will scan a sizable portion of the descriptors table if the request is large enough. This makes no attempt to describe system information which does not have a descriptor. It will describe system tables which appear in the descriptors table, but it will not try to explain "tables" which do not have descriptors (example tsdb), or any other information stored in the keyspace without a descriptor (PseudoTableIDs, GossipKeys for example). Throughout this work, many existing utilities were duplicated, and then un-duplicated (`keys.TableDataMin`, `roachpb.Span.Overlap`, etc). If you see anything that seems to already exist, feel free to point it out accordingly. Epic: CRDB-43151 Fixes: #130997 Release note: None --- pkg/BUILD.bazel | 2 + pkg/roachpb/BUILD.bazel | 2 + pkg/roachpb/data.go | 30 ++++ pkg/roachpb/key_test.go | 108 +++++++++++++ pkg/roachpb/span_test.go | 75 +++++++++ pkg/server/apiutil/BUILD.bazel | 33 +++- pkg/server/apiutil/rangeutil.go | 205 ++++++++++++++++++++++++ pkg/server/apiutil/rangeutil_test.go | 225 +++++++++++++++++++++++++++ pkg/sql/catalog/descs/collection.go | 21 ++- 9 files changed, 695 insertions(+), 6 deletions(-) create mode 100644 pkg/roachpb/key_test.go create mode 100644 pkg/roachpb/span_test.go create mode 100644 pkg/server/apiutil/rangeutil.go create mode 100644 pkg/server/apiutil/rangeutil_test.go diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 7b33745b9fd5..a9cc27ca439c 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -325,6 +325,7 @@ ALL_TESTS = [ "//pkg/security/username:username_disallowed_imports_test", "//pkg/security/username:username_test", "//pkg/security:security_test", + "//pkg/server/apiutil:apiutil_test", "//pkg/server/application_api:application_api_test", "//pkg/server/authserver:authserver_test", "//pkg/server/debug/goroutineui:goroutineui_test", @@ -1662,6 +1663,7 @@ GO_TARGETS = [ "//pkg/security:security_test", "//pkg/server/apiconstants:apiconstants", "//pkg/server/apiutil:apiutil", + "//pkg/server/apiutil:apiutil_test", "//pkg/server/application_api:application_api", "//pkg/server/application_api:application_api_test", "//pkg/server/authserver:authserver", diff --git a/pkg/roachpb/BUILD.bazel b/pkg/roachpb/BUILD.bazel index e8f4eed43884..92d96122981a 100644 --- a/pkg/roachpb/BUILD.bazel +++ b/pkg/roachpb/BUILD.bazel @@ -60,6 +60,7 @@ go_test( srcs = [ "data_test.go", "index_usage_stats_test.go", + "key_test.go", "main_test.go", "merge_spans_test.go", "metadata_replicas_test.go", @@ -67,6 +68,7 @@ go_test( "span_config_conformance_report_test.go", "span_config_test.go", "span_group_test.go", + "span_test.go", "string_test.go", "tenant_test.go", "version_test.go", diff --git a/pkg/roachpb/data.go b/pkg/roachpb/data.go index 5ad9ad1b19fe..7ec6470a9745 100644 --- a/pkg/roachpb/data.go +++ b/pkg/roachpb/data.go @@ -218,6 +218,23 @@ func (k Key) Compare(b Key) int { return bytes.Compare(k, b) } +// Less says whether key k is less than key b. +func (k Key) Less(b Key) bool { + return k.Compare(b) < 0 +} + +// Clamp fixes the key to something within the range a < k < b. +func (k Key) Clamp(a, b Key) Key { + result := k + if k.Less(a) { + result = a + } + if b.Less(k) { + result = b + } + return result +} + // SafeFormat implements the redact.SafeFormatter interface. func (k Key) SafeFormat(w redact.SafePrinter, _ rune) { SafeFormatKey(w, nil /* valDirs */, k) @@ -2360,6 +2377,19 @@ func (s Span) Equal(o Span) bool { return s.Key.Equal(o.Key) && s.EndKey.Equal(o.EndKey) } +// ZeroLength returns true if the distance between the start and end key is 0. +func (s Span) ZeroLength() bool { + return s.Key.Equal(s.EndKey) +} + +// Clamp clamps span s's keys within the span defined in bounds. +func (s Span) Clamp(bounds Span) Span { + return Span{ + s.Key.Clamp(bounds.Key, bounds.EndKey), + s.EndKey.Clamp(bounds.Key, bounds.EndKey), + } +} + // Overlaps returns true WLOG for span A and B iff: // 1. Both spans contain one key (just the start key) and they are equal; or // 2. The span with only one key is contained inside the other span; or diff --git a/pkg/roachpb/key_test.go b/pkg/roachpb/key_test.go new file mode 100644 index 000000000000..3cdb9406084f --- /dev/null +++ b/pkg/roachpb/key_test.go @@ -0,0 +1,108 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +package roachpb_test + +import ( + "math" + "testing" + + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/util/encoding" +) + +func TestKeyClampTenants(t *testing.T) { + // tp = TablePrefix + tp := keys.MakeSQLCodec(roachpb.MustMakeTenantID(3)).TablePrefix + lowTp := keys.MakeSQLCodec(roachpb.MustMakeTenantID(1)).TablePrefix + highTp := keys.MakeSQLCodec(roachpb.MustMakeTenantID(5)).TablePrefix + sysTp := keys.SystemSQLCodec.TablePrefix + tests := []struct { + name string + k, a, b roachpb.Key + expected roachpb.Key + }{ + {"key within main tenant is unchanged", tp(5), tp(1), tp(10), tp(5)}, + {"low tenant codec gets clamped to lower bound", lowTp(5), tp(1), tp(10), tp(1)}, + {"high tenant codec gets clamped to upper bound", highTp(5), tp(1), tp(10), tp(10)}, + {"system codec occurs below the tenant table boundaries", sysTp(5), tp(1), tp(10), tp(1)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.k.Clamp(tt.a, tt.b) + if !result.Equal(tt.expected) { + t.Errorf("Clamp(%v, %v, %v) = %v; want %v", tt.k, tt.a, tt.b, result, tt.expected) + } + }) + } +} + +func TestKeyClampTables(t *testing.T) { + // tp = TablePrefix + tp := keys.MakeSQLCodec(roachpb.MustMakeTenantID(3)).TablePrefix + tests := []struct { + name string + k, a, b roachpb.Key + expected roachpb.Key + }{ + {"table within prefix is unchanged", tp(5), tp(1), tp(10), tp(5)}, + {"low table gets clamped to lower bound", tp(0), tp(1), tp(10), tp(1)}, + {"high table gets clamped to upper bound", tp(11), tp(1), tp(10), tp(10)}, + {"low table on lower bound is unchanged", tp(1), tp(1), tp(10), tp(1)}, + {"high table on upper bound is unchanged", tp(10), tp(1), tp(10), tp(10)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.k.Clamp(tt.a, tt.b) + if !result.Equal(tt.expected) { + t.Errorf("Clamp(%v, %v, %v) = %v; want %v", tt.k, tt.a, tt.b, result, tt.expected) + } + }) + } +} + +func TestKeyClampTenantTablespace(t *testing.T) { + timeseriesKeyPrefix := encoding.EncodeVarintAscending( + encoding.EncodeBytesAscending( + append(roachpb.Key(nil), keys.TimeseriesPrefix...), + []byte("my.fake.metric"), + ), + int64(10), + ) + tsKey := func(source string, timestamp int64) roachpb.Key { + return append(encoding.EncodeVarintAscending(timeseriesKeyPrefix, timestamp), source...) + } + + tp := keys.MakeSQLCodec(roachpb.MustMakeTenantID(3)).TablePrefix + lower := tp(0) + upper := tp(math.MaxUint32) + tests := []struct { + name string + k, a, b roachpb.Key + expected roachpb.Key + }{ + {"KeyMin gets clamped to lower", roachpb.KeyMin, lower, upper, lower}, + {"KeyMax gets clamped to upper", roachpb.KeyMax, lower, upper, upper}, + {"Meta1Prefix gets clamped to lower", keys.Meta1Prefix, lower, upper, lower}, + {"Meta2Prefix gets clamped to lower", keys.Meta2Prefix, lower, upper, lower}, + {"TableDataMin gets clamped to lower", keys.TableDataMin, lower, upper, lower}, + // below is an unexpected test case for a tenant codec + {"TableDataMax also gets clamped to lower", keys.TableDataMax, lower, upper, lower}, + {"SystemPrefix gets clamped to lower", keys.SystemPrefix, lower, upper, lower}, + {"TimeseriesKey gets clamped to lower", tsKey("5", 123), lower, upper, lower}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.k.Clamp(tt.a, tt.b) + if !result.Equal(tt.expected) { + t.Errorf("Clamp(%v, %v, %v) = %v; want %v", tt.k, tt.a, tt.b, result, tt.expected) + } + }) + } +} diff --git a/pkg/roachpb/span_test.go b/pkg/roachpb/span_test.go new file mode 100644 index 000000000000..a6214419446a --- /dev/null +++ b/pkg/roachpb/span_test.go @@ -0,0 +1,75 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +package roachpb_test + +import ( + "testing" + + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/roachpb" +) + +func TestSpanZeroLength(t *testing.T) { + // create two separate references here. + shouldBeEmpty := roachpb.Span{ + Key: keys.SystemSQLCodec.TablePrefix(1), + EndKey: keys.SystemSQLCodec.TablePrefix(1), + } + if !shouldBeEmpty.ZeroLength() { + t.Fatalf("expected span %s to be empty.", shouldBeEmpty) + } + + shouldNotBeEmpty := roachpb.Span{ + Key: keys.SystemSQLCodec.TablePrefix(1), + EndKey: keys.SystemSQLCodec.TablePrefix(1).Next(), + } + if shouldNotBeEmpty.ZeroLength() { + t.Fatalf("expected span %s to not be empty.", shouldNotBeEmpty) + } +} + +func TestSpanClamp(t *testing.T) { + tp := keys.SystemSQLCodec.TablePrefix + tests := []struct { + name string + span roachpb.Span + bounds roachpb.Span + want roachpb.Span + }{ + { + name: "within bounds", + span: roachpb.Span{tp(5), tp(10)}, + bounds: roachpb.Span{tp(0), tp(15)}, + want: roachpb.Span{tp(5), tp(10)}, + }, + { + name: "clamp lower bound", + span: roachpb.Span{tp(0), tp(10)}, + bounds: roachpb.Span{tp(5), tp(15)}, + want: roachpb.Span{tp(5), tp(10)}, + }, + { + name: "clamp upper bound", + span: roachpb.Span{tp(5), tp(20)}, + bounds: roachpb.Span{tp(0), tp(15)}, + want: roachpb.Span{tp(5), tp(15)}, + }, + { + name: "clamp both bounds", + span: roachpb.Span{tp(0), tp(20)}, + bounds: roachpb.Span{tp(5), tp(15)}, + want: roachpb.Span{tp(5), tp(15)}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.span.Clamp(tt.bounds); !got.Equal(tt.want) { + t.Errorf("Clamp() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/server/apiutil/BUILD.bazel b/pkg/server/apiutil/BUILD.bazel index 0d6bba28b829..74fa6d52869c 100644 --- a/pkg/server/apiutil/BUILD.bazel +++ b/pkg/server/apiutil/BUILD.bazel @@ -1,9 +1,36 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "apiutil", - srcs = ["apiutil.go"], + srcs = [ + "apiutil.go", + "rangeutil.go", + ], importpath = "github.com/cockroachdb/cockroach/pkg/server/apiutil", visibility = ["//visibility:public"], - deps = ["//pkg/server/srverrors"], + deps = [ + "//pkg/keys", + "//pkg/roachpb", + "//pkg/server/srverrors", + "//pkg/sql/catalog", + "//pkg/sql/catalog/descpb", + "//pkg/sql/catalog/descs", + "@com_github_cockroachdb_errors//:errors", + ], +) + +go_test( + name = "apiutil_test", + srcs = ["rangeutil_test.go"], + deps = [ + ":apiutil", + "//pkg/keys", + "//pkg/roachpb", + "//pkg/sql/catalog", + "//pkg/sql/catalog/dbdesc", + "//pkg/sql/catalog/descpb", + "//pkg/sql/catalog/tabledesc", + "//pkg/sql/sem/catid", + "@com_github_stretchr_testify//require", + ], ) diff --git a/pkg/server/apiutil/rangeutil.go b/pkg/server/apiutil/rangeutil.go new file mode 100644 index 000000000000..b80fb9ed3f7d --- /dev/null +++ b/pkg/server/apiutil/rangeutil.go @@ -0,0 +1,205 @@ +// Copyright 2023 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +package apiutil + +import ( + "context" + "math" + + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" + "github.com/cockroachdb/errors" +) + +// This file contains a set of helper functions which are useful for turning +// ranges into the SQL related contents which reside within. It includes: +// 1. A utility function which turns a range into a span, and clamps it +// to its tenant's table space. +// 2. A utility function which takes the above spans and uses the catalog +// and new descriptor by span utility to turn those spans into a set of +// table descriptors ordered by id. +// 3. A utility function which transforms those table descriptors into a +// set of (database, table, index) names which deduplicate and identify +// each index uniquely. +// 4. A utility function, which merges the ranges and indexes into a map +// keyed by RangeID whose values are the above index names. +// 5. A primary entrypoint for consumers from which a set of ranges can be +// passed in and a mapping from those ranges to indexes can be +// returned. + +// IndexNames is a set of identifiers for an index span. +// It includes the database and table from the span's four part name +// as well as the index name itself. +type IndexNames struct { + Database string + Table string + Index string + Span roachpb.Span +} + +// Equal only compares the names, not the spans +func (idx IndexNames) Equal(other IndexNames) bool { + return idx.Database == other.Database && + idx.Table == other.Table && + idx.Index == other.Index +} + +// GetRangeIndexMappings translates a set of ordered ranges into a +// RangeID -> []IndexNames mapping. It does this by executing the fololowing steps: +// 1. Convert the set of ranges to a set of spans. +// 2. Get the table descriptors that fall within the given spans. +// 3. Get the database, table and index name for all indexes found in the descriptors. +// 4. Return a mapping of the indexes which appear in each range. +func GetRangeIndexMapping( + ctx context.Context, + txn descs.Txn, + codec keys.SQLCodec, + databases map[descpb.ID]catalog.DatabaseDescriptor, + ranges []roachpb.RangeDescriptor, +) (map[roachpb.RangeID][]IndexNames, error) { + spans := RangesToTableSpans(codec, ranges) + + tables, err := SpansToOrderedTableDescriptors(ctx, txn, spans) + if err != nil { + return nil, err + } + + indexes, err := TableDescriptorsToIndexNames(codec, databases, tables) + if err != nil { + return nil, err + } + + return MapRangesToIndexes(ranges, indexes), nil +} + +// MapRangesToIndexes is a utility function which iterates over two lists, +// one consisting of ordered ranges, and the other consisting of ordered index names +// and outputs a mapping from range to index. +func MapRangesToIndexes( + ranges []roachpb.RangeDescriptor, indexes []IndexNames, +) map[roachpb.RangeID][]IndexNames { + results := map[roachpb.RangeID][]IndexNames{} + contents := []IndexNames{} + flushToResults := func(rangeID roachpb.RangeID) { + results[rangeID] = contents + contents = []IndexNames{} + } + + // move through the ranges + descriptors + // using two indexes, i, j. + // while i and j are valid + i := 0 + j := 0 + for i < len(ranges) && j < len(indexes) { + rangeSpan := ranges[i].KeySpan().AsRawSpanWithNoLocals() + if rangeSpan.Overlaps(indexes[j].Span) { + contents = append(contents, indexes[j]) + } + + if ranges[i].EndKey.AsRawKey().Less(indexes[j].Span.EndKey) { + flushToResults(ranges[i].RangeID) + i++ + } else { + j++ + } + } + + if i < len(ranges) { + flushToResults(ranges[i].RangeID) + } + return results +} + +// RangeToTableSpans is a simple utility function which converts a set of ranges +// to a set of spans bound to the codec's SQL table space, and removed if the bound +// span is zero length. +func RangesToTableSpans(codec keys.SQLCodec, ranges []roachpb.RangeDescriptor) []roachpb.Span { + spans := []roachpb.Span{} + + // cannot use keys.TableDataMin/Max + // Check the following: keys.TableDataMax.Less(keys.MakeSQLCodec(3).TablePrefix(1)) == true + bounds := roachpb.Span{ + Key: codec.TablePrefix(0), + EndKey: codec.TablePrefix(math.MaxUint32), + } + for _, rangeDesc := range ranges { + span := rangeDesc.KeySpan().AsRawSpanWithNoLocals().Clamp(bounds) + if !span.ZeroLength() { + spans = append(spans, span) + } + } + + return spans +} + +// SpansToOrderedTableDescriptors uses the transaction's collection to turn a set of +// spans to a set of descriptors which describe the table space in which those spans lie. +func SpansToOrderedTableDescriptors( + ctx context.Context, txn descs.Txn, spans []roachpb.Span, +) ([]catalog.TableDescriptor, error) { + descriptors := []catalog.TableDescriptor{} + collection := txn.Descriptors() + nscatalog, err := collection.GetDescriptorsInSpans(ctx, txn.KV(), spans) + if err != nil { + return nil, err + } + + allDescriptors := nscatalog.OrderedDescriptors() + for _, iDescriptor := range allDescriptors { + if table, ok := iDescriptor.(catalog.TableDescriptor); ok { + descriptors = append(descriptors, table) + } + } + return descriptors, nil +} + +// TableDescriptorsToIndexNames is a simple function which maps a set of descriptors to the +// database, table, index combinations within. It assumes that every table +// has at least one index, the descriptors input are ordered, and that +// there can be duplicates of the descriptors. +func TableDescriptorsToIndexNames( + codec keys.SQLCodec, + databases map[descpb.ID]catalog.DatabaseDescriptor, + tables []catalog.TableDescriptor, +) ([]IndexNames, error) { + seen := map[string]struct{}{} + indexes := []IndexNames{} + + for _, table := range tables { + database, ok := databases[table.GetParentID()] + if !ok { + return nil, errors.Errorf("could not find database for table %s", table.GetName()) + } + for _, index := range table.AllIndexes() { + key := database.GetName() + table.GetName() + index.GetName() + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + indexes = append(indexes, IndexNames{ + Database: database.GetName(), + Table: table.GetName(), + Index: index.GetName(), + Span: spanFromIndex(codec, table, index), + }) + } + } + + return indexes, nil +} + +func spanFromIndex( + codec keys.SQLCodec, table catalog.TableDescriptor, index catalog.Index, +) roachpb.Span { + prefix := codec.IndexPrefix(uint32(table.GetID()), uint32(index.GetID())) + return roachpb.Span{ + Key: prefix, + EndKey: prefix.PrefixEnd(), + } +} diff --git a/pkg/server/apiutil/rangeutil_test.go b/pkg/server/apiutil/rangeutil_test.go new file mode 100644 index 000000000000..79f704ac3fcd --- /dev/null +++ b/pkg/server/apiutil/rangeutil_test.go @@ -0,0 +1,225 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +package apiutil_test + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/cockroachdb/cockroach/pkg/keys" + "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/server/apiutil" + "github.com/cockroachdb/cockroach/pkg/sql/catalog" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc" + "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" + "github.com/stretchr/testify/require" +) + +func TestMapRangesToIndexes(t *testing.T) { + k := func(c byte) roachpb.Key { + return roachpb.Key([]byte{c}) + } + rk := func(c byte) roachpb.RKey { + return roachpb.RKey(k(c)) + } + ranges := []roachpb.RangeDescriptor{ + {RangeID: 1, StartKey: rk('d'), EndKey: rk('m')}, + {RangeID: 2, StartKey: rk('m'), EndKey: rk('x')}, + } + + indexes := []apiutil.IndexNames{ + {Index: "Totally before first", Span: roachpb.Span{Key: k('a'), EndKey: k('c')}}, + {Index: "Start and before first", Span: roachpb.Span{Key: k('c'), EndKey: k('e')}}, + {Index: "Middle of first range", Span: roachpb.Span{Key: k('e'), EndKey: k('f')}}, + {Index: "Overlaps with both", Span: roachpb.Span{Key: k('f'), EndKey: k('o')}}, + {Index: "Middle of second range", Span: roachpb.Span{Key: k('o'), EndKey: k('q')}}, + {Index: "End and after second", Span: roachpb.Span{Key: k('q'), EndKey: k('y')}}, + {Index: "Totally after end", Span: roachpb.Span{Key: k('y'), EndKey: k('z')}}, + } + + expected := map[roachpb.RangeID][]apiutil.IndexNames{ + 1: { + {Index: "Start and before first"}, + {Index: "Middle of first range"}, + {Index: "Overlaps with both"}, + }, + 2: { + {Index: "Overlaps with both"}, + {Index: "Middle of second range"}, + {Index: "End and after second"}, + }, + } + + result := apiutil.MapRangesToIndexes(ranges, indexes) + + require.Equal(t, len(result), len(expected)) + b, _ := json.MarshalIndent(result, "", "\t") + fmt.Println(string(b)) + b, _ = json.MarshalIndent(indexes, "", "\t") + fmt.Println(string(b)) + + for rangeID, expectedIndexes := range expected { + actualIndexes, ok := result[rangeID] + require.True(t, ok) + require.Equal(t, len(actualIndexes), len(expectedIndexes)) + for i, expectedIndex := range expectedIndexes { + fmt.Println(rangeID, i, expectedIndex, actualIndexes[i]) + require.Equal(t, expectedIndex.Index, actualIndexes[i].Index) + } + } +} + +func TestRangesToTableSpans(t *testing.T) { + codec := keys.MakeSQLCodec(roachpb.MustMakeTenantID(1)) + ranges := []roachpb.RangeDescriptor{ + // should be zero len + { + StartKey: roachpb.RKey(codec.TablePrefix(0).Prevish(1)), + EndKey: roachpb.RKey(codec.TablePrefix(0)), + }, + // should also be zero len + { + StartKey: roachpb.RKey(codec.TablePrefix(1)), + EndKey: roachpb.RKey(codec.TablePrefix(1)), + }, + { + StartKey: roachpb.RKey(codec.TablePrefix(1)), + EndKey: roachpb.RKey(codec.TablePrefix(3)), + }, + { + StartKey: roachpb.RKey(codec.TablePrefix(3)), + EndKey: roachpb.RKey(codec.TablePrefix(5)), + }, + { + StartKey: roachpb.RKey(codec.TablePrefix(5)), + EndKey: roachpb.RKey(codec.TablePrefix(6)), + }, + } + + expected := []roachpb.Span{ + { + Key: codec.TablePrefix(1), + EndKey: codec.TablePrefix(3), + }, + { + Key: codec.TablePrefix(3), + EndKey: codec.TablePrefix(5), + }, + { + Key: codec.TablePrefix(5), + EndKey: codec.TablePrefix(6), + }, + } + + result := apiutil.RangesToTableSpans(codec, ranges) + + require.Equal(t, len(result), len(expected)) + + for i, span := range expected { + if !result[i].Equal(span) { + t.Fatalf("expected span %v, got %v", span, result[i]) + } + } +} + +func makeDBDesc(id uint32, name string) catalog.DatabaseDescriptor { + + db := &dbdesc.Mutable{} + db.SetName(name) + db.ID = catid.DescID(id) + descriptor := db.ImmutableCopy() + return descriptor.(catalog.DatabaseDescriptor) +} + +func makeTableDesc(databaseID uint32, name string, indexes []string) catalog.TableDescriptor { + descIndexes := []descpb.IndexDescriptor{} + table := tabledesc.NewBuilder(&descpb.TableDescriptor{ + Name: name, + ParentID: catid.DescID(databaseID), + Indexes: descIndexes, + }).BuildCreatedMutableTable() + for i, name := range indexes { + if i == 0 { + _ = table.AddPrimaryIndex(descpb.IndexDescriptor{Name: name}) + } else { + + _ = table.AddSecondaryIndex(descpb.IndexDescriptor{Name: name}) + } + } + return table.NewBuilder().BuildImmutable().(catalog.TableDescriptor) +} + +func TestTableDescriptorsToIndexNames(t *testing.T) { + // test straightforward path with three tables, two databases + codec := keys.MakeSQLCodec(roachpb.MustMakeTenantID(1)) + databases := map[descpb.ID]catalog.DatabaseDescriptor{ + 1: makeDBDesc(1, "db1"), + 2: makeDBDesc(2, "db2"), + } + tables := []catalog.TableDescriptor{ + makeTableDesc(1, "table1", []string{"pkey"}), + makeTableDesc(2, "table2", []string{"pkey", "table2_secondary_column"}), + makeTableDesc(2, "table3", []string{"pkey"}), + } + + expected := []apiutil.IndexNames{ + {Database: "db1", Table: "table1", Index: "pkey"}, + {Database: "db2", Table: "table2", Index: "pkey"}, + {Database: "db2", Table: "table2", Index: "table2_secondary_column"}, + {Database: "db2", Table: "table3", Index: "pkey"}, + } + indexes, err := apiutil.TableDescriptorsToIndexNames(codec, databases, tables) + + require.NoError(t, err) + require.Equal(t, len(expected), len(indexes)) + for i, index := range indexes { + if !index.Equal(expected[i]) { + t.Fatalf("resulting index did not match expected output: %s %s", index, expected[i]) + } + } +} + +func TestTableDescriptorsToIndexNamesDeduplicates(t *testing.T) { + // verify that duplicate descriptors are de-duplicated + codec := keys.MakeSQLCodec(roachpb.MustMakeTenantID(1)) + databases := map[descpb.ID]catalog.DatabaseDescriptor{ + 1: makeDBDesc(1, "db1"), + } + tables := []catalog.TableDescriptor{ + makeTableDesc(1, "table1", []string{"pkey", "table1_secondary_column"}), + makeTableDesc(1, "table1", []string{"pkey", "table1_secondary_column"}), + } + + expected := []apiutil.IndexNames{ + {Database: "db1", Table: "table1", Index: "pkey"}, + {Database: "db1", Table: "table1", Index: "table1_secondary_column"}, + } + indexes, err := apiutil.TableDescriptorsToIndexNames(codec, databases, tables) + + require.NoError(t, err) + require.Equal(t, len(expected), len(indexes)) + for i, index := range indexes { + if !index.Equal(expected[i]) { + t.Fatalf("resulting index did not match expected output: %s %s", index, expected[i]) + } + } +} + +func TestGetIndexNamesFromDescriptorsMissingDatabase(t *testing.T) { + codec := keys.MakeSQLCodec(roachpb.MustMakeTenantID(1)) + databases := map[descpb.ID]catalog.DatabaseDescriptor{ + 1: makeDBDesc(1, "db1"), + } + tables := []catalog.TableDescriptor{ + makeTableDesc(2, "table2", []string{"pkey", "table2_secondary_column"}), + } + + _, err := apiutil.TableDescriptorsToIndexNames(codec, databases, tables) + require.Errorf(t, err, "could not find database for table %s", tables[0].GetName()) +} diff --git a/pkg/sql/catalog/descs/collection.go b/pkg/sql/catalog/descs/collection.go index 898db4063400..61fabafc93ee 100644 --- a/pkg/sql/catalog/descs/collection.go +++ b/pkg/sql/catalog/descs/collection.go @@ -1132,9 +1132,24 @@ func (tc *Collection) GetAllDatabaseDescriptors( return ret, nil } -// GetSchemasForDatabase returns the schemas for a given database -// visible by the transaction. -// Deprecated: prefer GetAllSchemasInDatabase. +// GetAllDatabaseDescriptorsMap returns the results of GetAllDatabaseDescriptors +func (tc *Collection) GetAllDatabaseDescriptorsMap( + ctx context.Context, txn *kv.Txn, +) (ret map[descpb.ID]catalog.DatabaseDescriptor, _ error) { + descriptors, err := tc.GetAllDatabaseDescriptors(ctx, txn) + result := map[descpb.ID]catalog.DatabaseDescriptor{} + if err != nil { + return nil, err + } + + for _, descriptor := range descriptors { + result[descriptor.GetID()] = descriptor + } + + return result, nil +} + +// but as a map with the database ID as the key. func (tc *Collection) GetSchemasForDatabase( ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor, ) (map[descpb.ID]string, error) { From 8227435bb14ee75252af73bfb002cd08548b30ac Mon Sep 17 00:00:00 2001 From: Brian Dillmann Date: Wed, 30 Oct 2024 09:58:04 -0400 Subject: [PATCH 3/4] apiutil, roachpb: create utilities to map descriptors to ranges Previously each range correlated to a single table, or even a single index in a database, so all that was required to identify which tables, indexes were in the range were to look at the start key of the range and map it accordingly. With range coalescing however, it's possible for one, or many, tables, indexes and the like to reside within the same range. To properly identify the contents of a range, this PR adds the following utilities: 1. A utility function which turns a range into a span, and clamps it to its tenant's table space. 2. A utility function which takes the above spans and uses the catalog and new descriptor by span utility to turn those spans into a set of table descriptors ordered by id. 3. A utility function which transforms those table descriptors into a set of (database, table, index) names which deduplicate and identify each index uniquely. 4. A utility function, which merges the ranges and indexes into a map keyed by RangeID whose values are the above index names. 5. A primary entrypoint for consumers from which a set of ranges can be passed in and a mapping from those ranges to indexes can be returned. A variety of cavets come with this approach. It attempts to scan the desciptors all at once, but it still will scan a sizable portion of the descriptors table if the request is large enough. This makes no attempt to describe system information which does not have a descriptor. It will describe system tables which appear in the descriptors table, but it will not try to explain "tables" which do not have descriptors (example tsdb), or any other information stored in the keyspace without a descriptor (PseudoTableIDs, GossipKeys for example). Throughout this work, many existing utilities were duplicated, and then un-duplicated (`keys.TableDataMin`, `roachpb.Span.Overlap`, etc). If you see anything that seems to already exist, feel free to point it out accordingly. Epic: none Fixes: #130997 Release note: None --- pkg/roachpb/metadata.go | 15 +++++++++++++++ pkg/roachpb/metadata_test.go | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/pkg/roachpb/metadata.go b/pkg/roachpb/metadata.go index 26cfc941408d..3e7dde5ff673 100644 --- a/pkg/roachpb/metadata.go +++ b/pkg/roachpb/metadata.go @@ -1006,3 +1006,18 @@ func (h *GCHint) advanceGCTimestamp(gcThreshold hlc.Timestamp) bool { h.GCTimestamp, h.GCTimestampNext = hlc.Timestamp{}, hlc.Timestamp{} return true } + +type RangeDescriptorsByStartKey []RangeDescriptor + +func (r RangeDescriptorsByStartKey) Len() int { + return len(r) +} +func (r RangeDescriptorsByStartKey) Less(i, j int) bool { + return r[i].StartKey.AsRawKey().Less(r[j].StartKey.AsRawKey()) +} + +func (r RangeDescriptorsByStartKey) Swap(i, j int) { + tmp := r[i] + r[i] = r[j] + r[j] = tmp +} diff --git a/pkg/roachpb/metadata_test.go b/pkg/roachpb/metadata_test.go index 6970c44dc23e..77fabc4ecde5 100644 --- a/pkg/roachpb/metadata_test.go +++ b/pkg/roachpb/metadata_test.go @@ -8,6 +8,7 @@ package roachpb import ( "fmt" "reflect" + "sort" "strings" "testing" @@ -662,3 +663,24 @@ func TestGCHint(t *testing.T) { }) } } + +func TestRangeDescriptorsByStartKey(t *testing.T) { + // table-prefix-range-key + tprk := func(t byte) RKey { + return RKey(Key([]byte{t})) + } + ranges := []RangeDescriptor{ + {StartKey: tprk(2), EndKey: tprk(7)}, + {StartKey: tprk(5), EndKey: tprk(5)}, + {StartKey: tprk(7), EndKey: tprk(2)}, + {StartKey: tprk(1), EndKey: tprk(10)}, + {StartKey: tprk(5), EndKey: tprk(5)}, + } + sort.Stable(RangeDescriptorsByStartKey(ranges)) + + for i := 0; i < len(ranges)-1; i++ { + if ranges[i+1].StartKey.AsRawKey().Less(ranges[i].StartKey.AsRawKey()) { + t.Fatalf("expected ranges to be ordered increasing by start key, failed on %d, %d with keys %s, %s", i, i+1, ranges[i].StartKey.AsRawKey(), ranges[i+1].StartKey.AsRawKey()) + } + } +} From 67c45eb3d430eba71c70259e02ca89a0bd7c9bca Mon Sep 17 00:00:00 2001 From: Brian Dillmann Date: Fri, 1 Nov 2024 16:28:04 -0400 Subject: [PATCH 4/4] ui, server: modify hot ranges api and table to use new contents approx. This change is the last in a set of commits to change the hot ranges page from only showing one table, index per range to many. It builds on top of the changes in the catalog reader (10b9ee03fba37f314771d8624b2619f10b5bbb59) and the range utilities (109219d5fca6d347663127bf21b8dd162ef51d4a) to surface a set of tables, indexes for each range. The primary changes in this commit specifically are the modification of the status server to use the new `rangeutil` utilities, and changing the wire, presentation format of the information. Epic: CRDB-43151 Fixes: #130997 Release note (bug fix): changes the table, index contents of the hot ranges page in DB console. --- docs/generated/http/full.md | 9 +- pkg/roachpb/BUILD.bazel | 1 + pkg/roachpb/data.go | 31 +++-- pkg/roachpb/key_test.go | 25 +++- pkg/roachpb/span_test.go | 17 ++- pkg/server/BUILD.bazel | 1 - pkg/server/apiutil/BUILD.bazel | 6 +- pkg/server/apiutil/index_names.go | 91 ++++++++++++++ pkg/server/apiutil/index_names_test.go | 119 ++++++++++++++++++ pkg/server/apiutil/rangeutil.go | 82 ++++++------ pkg/server/apiutil/rangeutil_test.go | 3 +- pkg/server/serverpb/status.proto | 14 ++- pkg/server/status.go | 112 ++++------------- pkg/sql/catalog/descs/collection.go | 10 +- .../catalog/internal/catkv/catalog_reader.go | 9 +- .../src/views/hotRanges/hotRangesTable.tsx | 25 +--- 16 files changed, 374 insertions(+), 181 deletions(-) create mode 100644 pkg/server/apiutil/index_names.go create mode 100644 pkg/server/apiutil/index_names_test.go diff --git a/docs/generated/http/full.md b/docs/generated/http/full.md index fabce1b5539f..31d96353028d 100644 --- a/docs/generated/http/full.md +++ b/docs/generated/http/full.md @@ -3653,9 +3653,9 @@ HotRange message describes a single hot range, ie its QPS, node ID it belongs to | range_id | [int32](#cockroach.server.serverpb.HotRangesResponseV2-int32) | | range_id indicates Range ID that's identified as hot range. | [reserved](#support-status) | | node_id | [int32](#cockroach.server.serverpb.HotRangesResponseV2-int32) | | node_id indicates the node that contains the current hot range. | [reserved](#support-status) | | qps | [double](#cockroach.server.serverpb.HotRangesResponseV2-double) | | qps (queries per second) shows the amount of queries that interact with current range. | [reserved](#support-status) | -| table_name | [string](#cockroach.server.serverpb.HotRangesResponseV2-string) | | table_name indicates the SQL table that the range belongs to. | [reserved](#support-status) | -| database_name | [string](#cockroach.server.serverpb.HotRangesResponseV2-string) | | database_name indicates on database that has current hot range. | [reserved](#support-status) | -| index_name | [string](#cockroach.server.serverpb.HotRangesResponseV2-string) | | index_name indicates the index name for current range. | [reserved](#support-status) | +| table_name | [string](#cockroach.server.serverpb.HotRangesResponseV2-string) | | table_name has been deprecated in favor of tables = 16; | [reserved](#support-status) | +| database_name | [string](#cockroach.server.serverpb.HotRangesResponseV2-string) | | database_name has been deprecated in favor of databases = 17; | [reserved](#support-status) | +| index_name | [string](#cockroach.server.serverpb.HotRangesResponseV2-string) | | index_name has been deprecated in favor of indexes = 17; | [reserved](#support-status) | | replica_node_ids | [int32](#cockroach.server.serverpb.HotRangesResponseV2-int32) | repeated | replica_node_ids specifies the list of node ids that contain replicas with current hot range. | [reserved](#support-status) | | leaseholder_node_id | [int32](#cockroach.server.serverpb.HotRangesResponseV2-int32) | | leaseholder_node_id indicates the Node ID that is the current leaseholder for the given range. | [reserved](#support-status) | | schema_name | [string](#cockroach.server.serverpb.HotRangesResponseV2-string) | | schema_name provides the name of schema (if exists) for table in current range. | [reserved](#support-status) | @@ -3665,6 +3665,9 @@ HotRange message describes a single hot range, ie its QPS, node ID it belongs to | write_bytes_per_second | [double](#cockroach.server.serverpb.HotRangesResponseV2-double) | | write_bytes_per_second is the recent number of bytes written per second on this range. | [reserved](#support-status) | | read_bytes_per_second | [double](#cockroach.server.serverpb.HotRangesResponseV2-double) | | read_bytes_per_second is the recent number of bytes read per second on this range. | [reserved](#support-status) | | cpu_time_per_second | [double](#cockroach.server.serverpb.HotRangesResponseV2-double) | | CPU time (ns) per second is the recent cpu usage per second on this range. | [reserved](#support-status) | +| databases | [string](#cockroach.server.serverpb.HotRangesResponseV2-string) | repeated | Databases for the range. | [reserved](#support-status) | +| tables | [string](#cockroach.server.serverpb.HotRangesResponseV2-string) | repeated | Tables for the range | [reserved](#support-status) | +| indexes | [string](#cockroach.server.serverpb.HotRangesResponseV2-string) | repeated | Indexes for the range | [reserved](#support-status) | diff --git a/pkg/roachpb/BUILD.bazel b/pkg/roachpb/BUILD.bazel index 92d96122981a..c2a6d7637475 100644 --- a/pkg/roachpb/BUILD.bazel +++ b/pkg/roachpb/BUILD.bazel @@ -84,6 +84,7 @@ go_test( "//pkg/raft/raftpb", "//pkg/raft/tracker", "//pkg/storage/enginepb", + "//pkg/testutils", "//pkg/testutils/zerofields", "//pkg/util", "//pkg/util/bitarray", diff --git a/pkg/roachpb/data.go b/pkg/roachpb/data.go index 7ec6470a9745..2ba82c4b135a 100644 --- a/pkg/roachpb/data.go +++ b/pkg/roachpb/data.go @@ -224,15 +224,18 @@ func (k Key) Less(b Key) bool { } // Clamp fixes the key to something within the range a < k < b. -func (k Key) Clamp(a, b Key) Key { +func (k Key) Clamp(min, max Key) (Key, error) { + if max.Less(min) { + return nil, errors.Newf("cannot clamp when min '%s' is larger than max '%s'", min, max) + } result := k - if k.Less(a) { - result = a + if k.Less(min) { + result = min } - if b.Less(k) { - result = b + if max.Less(k) { + result = max } - return result + return result, nil } // SafeFormat implements the redact.SafeFormatter interface. @@ -2383,11 +2386,19 @@ func (s Span) ZeroLength() bool { } // Clamp clamps span s's keys within the span defined in bounds. -func (s Span) Clamp(bounds Span) Span { - return Span{ - s.Key.Clamp(bounds.Key, bounds.EndKey), - s.EndKey.Clamp(bounds.Key, bounds.EndKey), +func (s Span) Clamp(bounds Span) (Span, error) { + start, err := s.Key.Clamp(bounds.Key, bounds.EndKey) + if err != nil { + return Span{}, err } + end, err := s.EndKey.Clamp(bounds.Key, bounds.EndKey) + if err != nil { + return Span{}, err + } + return Span{ + Key: start, + EndKey: end, + }, nil } // Overlaps returns true WLOG for span A and B iff: diff --git a/pkg/roachpb/key_test.go b/pkg/roachpb/key_test.go index 3cdb9406084f..55a2883ed78f 100644 --- a/pkg/roachpb/key_test.go +++ b/pkg/roachpb/key_test.go @@ -11,7 +11,9 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/util/encoding" + "github.com/stretchr/testify/require" ) func TestKeyClampTenants(t *testing.T) { @@ -33,7 +35,8 @@ func TestKeyClampTenants(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := tt.k.Clamp(tt.a, tt.b) + result, err := tt.k.Clamp(tt.a, tt.b) + require.NoError(t, err) if !result.Equal(tt.expected) { t.Errorf("Clamp(%v, %v, %v) = %v; want %v", tt.k, tt.a, tt.b, result, tt.expected) } @@ -58,7 +61,8 @@ func TestKeyClampTables(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := tt.k.Clamp(tt.a, tt.b) + result, err := tt.k.Clamp(tt.a, tt.b) + require.NoError(t, err) if !result.Equal(tt.expected) { t.Errorf("Clamp(%v, %v, %v) = %v; want %v", tt.k, tt.a, tt.b, result, tt.expected) } @@ -99,10 +103,25 @@ func TestKeyClampTenantTablespace(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := tt.k.Clamp(tt.a, tt.b) + result, err := tt.k.Clamp(tt.a, tt.b) + require.NoError(t, err) if !result.Equal(tt.expected) { t.Errorf("Clamp(%v, %v, %v) = %v; want %v", tt.k, tt.a, tt.b, result, tt.expected) } }) } } + +func TestKeyClampError(t *testing.T) { + // verify that max < min throws error + a, b := roachpb.Key([]byte{'a'}), roachpb.Key([]byte{'b'}) + expected := `cannot clamp when min '"b"' is larger than max '"a"'` + _, err := a.Clamp(b, a) + if !testutils.IsError(err, expected) { + t.Fatalf("expected error to be '%s', got '%s'", expected, err) + } + + // verify that max = min throws no error + _, err = a.Clamp(a, a) + require.NoError(t, err) +} diff --git a/pkg/roachpb/span_test.go b/pkg/roachpb/span_test.go index a6214419446a..baaecaf6ac02 100644 --- a/pkg/roachpb/span_test.go +++ b/pkg/roachpb/span_test.go @@ -10,6 +10,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/testutils" ) func TestSpanZeroLength(t *testing.T) { @@ -38,6 +39,7 @@ func TestSpanClamp(t *testing.T) { span roachpb.Span bounds roachpb.Span want roachpb.Span + error string }{ { name: "within bounds", @@ -63,12 +65,23 @@ func TestSpanClamp(t *testing.T) { bounds: roachpb.Span{tp(5), tp(15)}, want: roachpb.Span{tp(5), tp(15)}, }, + { + name: "clamp start error", + span: roachpb.Span{}, + bounds: roachpb.Span{tp(2), tp(1)}, + want: roachpb.Span{}, + error: "cannot clamp when min '/Table/2' is larger than max '/Table/1'", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := tt.span.Clamp(tt.bounds); !got.Equal(tt.want) { - t.Errorf("Clamp() = %v, want %v", got, tt.want) + span, err := tt.span.Clamp(tt.bounds) + if !testutils.IsError(err, tt.error) { + t.Fatalf("expected error to be '%s', got '%s'", tt.error, err) + } + if !span.Equal(tt.want) { + t.Errorf("Clamp() = %v, want %v", span, tt.want) } }) } diff --git a/pkg/server/BUILD.bazel b/pkg/server/BUILD.bazel index a22381b7c91f..5dbb99d311b8 100644 --- a/pkg/server/BUILD.bazel +++ b/pkg/server/BUILD.bazel @@ -210,7 +210,6 @@ go_library( "//pkg/sql", "//pkg/sql/appstatspb", "//pkg/sql/auditlogging", - "//pkg/sql/catalog", "//pkg/sql/catalog/bootstrap", "//pkg/sql/catalog/catalogkeys", "//pkg/sql/catalog/catsessiondata", diff --git a/pkg/server/apiutil/BUILD.bazel b/pkg/server/apiutil/BUILD.bazel index 74fa6d52869c..57a42fc48b17 100644 --- a/pkg/server/apiutil/BUILD.bazel +++ b/pkg/server/apiutil/BUILD.bazel @@ -4,6 +4,7 @@ go_library( name = "apiutil", srcs = [ "apiutil.go", + "index_names.go", "rangeutil.go", ], importpath = "github.com/cockroachdb/cockroach/pkg/server/apiutil", @@ -21,7 +22,10 @@ go_library( go_test( name = "apiutil_test", - srcs = ["rangeutil_test.go"], + srcs = [ + "index_names_test.go", + "rangeutil_test.go", + ], deps = [ ":apiutil", "//pkg/keys", diff --git a/pkg/server/apiutil/index_names.go b/pkg/server/apiutil/index_names.go new file mode 100644 index 000000000000..b68189171949 --- /dev/null +++ b/pkg/server/apiutil/index_names.go @@ -0,0 +1,91 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +package apiutil + +import ( + "fmt" + "strings" + + "github.com/cockroachdb/cockroach/pkg/roachpb" +) + +// IndexNames and IndexNamesList are utilities for representing an +// index span by its corresponding identifiers. +// They include the underlying span for comparison against other spans +// and omit the server and schema parts of their four part name. +type IndexNames struct { + Database string + Table string + Index string + Span roachpb.Span +} + +func (idx IndexNames) String() string { + return fmt.Sprintf("%s.%s.%s", idx.Database, idx.Table, idx.Index) +} + +type IndexNamesList []IndexNames + +// quoteIfContainsDot adds quotes to an identifier if it contains +// the four part delimiter '.'. +func quoteIfContainsDot(identifier string) string { + if strings.Contains(identifier, ".") { + return `"` + identifier + `"` + } + return identifier +} + +// ToOutput is a simple function which returns a set of +// de-duplicated, fully referenced database, table, and index names +// depending on the number of databases and tables which appear. +func (idxl IndexNamesList) ToOutput() ([]string, []string, []string) { + fpi := quoteIfContainsDot + seenDatabases := map[string]bool{} + databases := []string{} + for _, idx := range idxl { + database := fpi(idx.Database) + if !seenDatabases[database] { + seenDatabases[database] = true + databases = append(databases, database) + } + } + + multipleDatabases := len(databases) > 1 + seenTables := map[string]bool{} + tables := []string{} + for _, idx := range idxl { + table := fpi(idx.Table) + if multipleDatabases { + table = fpi(idx.Database) + "." + table + } + if !seenTables[table] { + seenTables[table] = true + tables = append(tables, table) + } + } + + multipleTables := len(tables) > 1 + indexes := []string{} + for _, idx := range idxl { + index := fpi(idx.Index) + if multipleTables { + index = fpi(idx.Table) + "." + index + if multipleDatabases { + index = fpi(idx.Database) + "." + index + } + } + indexes = append(indexes, index) + } + + return databases, tables, indexes +} + +// Equal only compares the names, not the spans +func (idx IndexNames) Equal(other IndexNames) bool { + return idx.Database == other.Database && + idx.Table == other.Table && + idx.Index == other.Index +} diff --git a/pkg/server/apiutil/index_names_test.go b/pkg/server/apiutil/index_names_test.go new file mode 100644 index 000000000000..b24ecfff5cd5 --- /dev/null +++ b/pkg/server/apiutil/index_names_test.go @@ -0,0 +1,119 @@ +// Copyright 2024 The Cockroach Authors. +// +// Use of this software is governed by the CockroachDB Software License +// included in the /LICENSE file. + +package apiutil_test + +import ( + "reflect" + "testing" + + "github.com/cockroachdb/cockroach/pkg/server/apiutil" +) + +func TestToOutput(t *testing.T) { + tests := []struct { + name string + input apiutil.IndexNamesList + expected struct { + databases []string + tables []string + indexes []string + } + }{ + { + name: "Single database, single table", + input: apiutil.IndexNamesList{ + {Database: "db1", Table: "table1", Index: "index1"}, + }, + expected: struct { + databases []string + tables []string + indexes []string + }{ + databases: []string{"db1"}, + tables: []string{"table1"}, + indexes: []string{"index1"}, + }, + }, + { + name: "Single database, multiple tables", + input: apiutil.IndexNamesList{ + {Database: "db1", Table: "table1", Index: "index1"}, + {Database: "db1", Table: "table2", Index: "index2"}, + }, + expected: struct { + databases []string + tables []string + indexes []string + }{ + databases: []string{"db1"}, + tables: []string{"table1", "table2"}, + indexes: []string{"table1.index1", "table2.index2"}, + }, + }, + { + name: "Multiple databases, multiple tables", + input: apiutil.IndexNamesList{ + {Database: "db1", Table: "table1", Index: "index1"}, + {Database: "db2", Table: "table2", Index: "index2"}, + }, + expected: struct { + databases []string + tables []string + indexes []string + }{ + databases: []string{"db1", "db2"}, + tables: []string{"db1.table1", "db2.table2"}, + indexes: []string{"db1.table1.index1", "db2.table2.index2"}, + }, + }, + { + name: "Duplicate entries", + input: apiutil.IndexNamesList{ + {Database: "db1", Table: "table1", Index: "index1"}, + {Database: "db1", Table: "table1", Index: "index1"}, + }, + expected: struct { + databases []string + tables []string + indexes []string + }{ + databases: []string{"db1"}, + tables: []string{"table1"}, + indexes: []string{"index1", "index1"}, + }, + }, + { + name: "Identifiers with dot in the name", + input: apiutil.IndexNamesList{ + {Database: "db.1", Table: "table.1", Index: "index1"}, + }, + expected: struct { + databases []string + tables []string + indexes []string + }{ + databases: []string{"\"db.1\""}, + tables: []string{"\"table.1\""}, + indexes: []string{"index1"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + databases, tables, indexes := tt.input.ToOutput() + if !reflect.DeepEqual(databases, tt.expected.databases) { + t.Errorf("expected databases %v, got %v", tt.expected.databases, databases) + } + if !reflect.DeepEqual(tables, tt.expected.tables) { + t.Errorf("expected tables %v, got %v", tt.expected.tables, tables) + } + if !reflect.DeepEqual(indexes, tt.expected.indexes) { + t.Errorf("expected indexes %v, got %v", tt.expected.indexes, indexes) + } + }) + } +} diff --git a/pkg/server/apiutil/rangeutil.go b/pkg/server/apiutil/rangeutil.go index b80fb9ed3f7d..e0269a39531e 100644 --- a/pkg/server/apiutil/rangeutil.go +++ b/pkg/server/apiutil/rangeutil.go @@ -1,4 +1,4 @@ -// Copyright 2023 The Cockroach Authors. +// Copyright 2024 The Cockroach Authors. // // Use of this software is governed by the CockroachDB Software License // included in the /LICENSE file. @@ -8,6 +8,7 @@ package apiutil import ( "context" "math" + "sort" "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" @@ -33,37 +34,33 @@ import ( // passed in and a mapping from those ranges to indexes can be // returned. -// IndexNames is a set of identifiers for an index span. -// It includes the database and table from the span's four part name -// as well as the index name itself. -type IndexNames struct { - Database string - Table string - Index string - Span roachpb.Span -} - -// Equal only compares the names, not the spans -func (idx IndexNames) Equal(other IndexNames) bool { - return idx.Database == other.Database && - idx.Table == other.Table && - idx.Index == other.Index +// rangeLess encapsulate a slice of ranges and returns a comparator function +// so that it can be sorted by go's builtin sort package. +func rangeLess(ranges []roachpb.RangeDescriptor) func(i, j int) bool { + return func(i, j int) bool { + return ranges[i].StartKey.Less(ranges[j].StartKey) + } } // GetRangeIndexMappings translates a set of ordered ranges into a -// RangeID -> []IndexNames mapping. It does this by executing the fololowing steps: -// 1. Convert the set of ranges to a set of spans. -// 2. Get the table descriptors that fall within the given spans. -// 3. Get the database, table and index name for all indexes found in the descriptors. -// 4. Return a mapping of the indexes which appear in each range. +// RangeID -> IndexNamesList mapping. It does this by executing the following steps: +// 1. Sort the incoming ranges by start key +// 2. Convert the set of ranges to a set of spans. +// 3. Get the table descriptors that fall within the given spans. +// 4. Get the database, table and index name for all indexes found in the descriptors. +// 5. Return a mapping of the indexes which appear in each range. func GetRangeIndexMapping( ctx context.Context, txn descs.Txn, codec keys.SQLCodec, databases map[descpb.ID]catalog.DatabaseDescriptor, ranges []roachpb.RangeDescriptor, -) (map[roachpb.RangeID][]IndexNames, error) { - spans := RangesToTableSpans(codec, ranges) +) (map[roachpb.RangeID]IndexNamesList, error) { + sort.Slice(ranges, rangeLess(ranges)) + spans, err := RangesToTableSpans(codec, ranges) + if err != nil { + return nil, err + } tables, err := SpansToOrderedTableDescriptors(ctx, txn, spans) if err != nil { @@ -82,13 +79,13 @@ func GetRangeIndexMapping( // one consisting of ordered ranges, and the other consisting of ordered index names // and outputs a mapping from range to index. func MapRangesToIndexes( - ranges []roachpb.RangeDescriptor, indexes []IndexNames, -) map[roachpb.RangeID][]IndexNames { - results := map[roachpb.RangeID][]IndexNames{} - contents := []IndexNames{} + ranges []roachpb.RangeDescriptor, indexes IndexNamesList, +) map[roachpb.RangeID]IndexNamesList { + results := map[roachpb.RangeID]IndexNamesList{} + contents := IndexNamesList{} flushToResults := func(rangeID roachpb.RangeID) { results[rangeID] = contents - contents = []IndexNames{} + contents = IndexNamesList{} } // move through the ranges + descriptors @@ -116,10 +113,12 @@ func MapRangesToIndexes( return results } -// RangeToTableSpans is a simple utility function which converts a set of ranges -// to a set of spans bound to the codec's SQL table space, and removed if the bound -// span is zero length. -func RangesToTableSpans(codec keys.SQLCodec, ranges []roachpb.RangeDescriptor) []roachpb.Span { +// RangeToTableSpans converts a set of ranges to a set of spans bound +// to the codec's SQL table space, and removed if the bound span is +// zero length. +func RangesToTableSpans( + codec keys.SQLCodec, ranges []roachpb.RangeDescriptor, +) ([]roachpb.Span, error) { spans := []roachpb.Span{} // cannot use keys.TableDataMin/Max @@ -129,13 +128,16 @@ func RangesToTableSpans(codec keys.SQLCodec, ranges []roachpb.RangeDescriptor) [ EndKey: codec.TablePrefix(math.MaxUint32), } for _, rangeDesc := range ranges { - span := rangeDesc.KeySpan().AsRawSpanWithNoLocals().Clamp(bounds) + span, err := rangeDesc.KeySpan().AsRawSpanWithNoLocals().Clamp(bounds) + if err != nil { + return nil, err + } if !span.ZeroLength() { spans = append(spans, span) } } - return spans + return spans, nil } // SpansToOrderedTableDescriptors uses the transaction's collection to turn a set of @@ -159,17 +161,17 @@ func SpansToOrderedTableDescriptors( return descriptors, nil } -// TableDescriptorsToIndexNames is a simple function which maps a set of descriptors to the -// database, table, index combinations within. It assumes that every table -// has at least one index, the descriptors input are ordered, and that -// there can be duplicates of the descriptors. +// TableDescriptorsToIndexNames maps a set of descriptors to the +// database, table, index combinations within. It assumes that every +// table has at least one index, the descriptors input are ordered, +// and that there can be duplicates of the descriptors. func TableDescriptorsToIndexNames( codec keys.SQLCodec, databases map[descpb.ID]catalog.DatabaseDescriptor, tables []catalog.TableDescriptor, -) ([]IndexNames, error) { +) (IndexNamesList, error) { seen := map[string]struct{}{} - indexes := []IndexNames{} + indexes := IndexNamesList{} for _, table := range tables { database, ok := databases[table.GetParentID()] diff --git a/pkg/server/apiutil/rangeutil_test.go b/pkg/server/apiutil/rangeutil_test.go index 79f704ac3fcd..4dabbea4a757 100644 --- a/pkg/server/apiutil/rangeutil_test.go +++ b/pkg/server/apiutil/rangeutil_test.go @@ -117,8 +117,9 @@ func TestRangesToTableSpans(t *testing.T) { }, } - result := apiutil.RangesToTableSpans(codec, ranges) + result, err := apiutil.RangesToTableSpans(codec, ranges) + require.NoError(t, err) require.Equal(t, len(result), len(expected)) for i, span := range expected { diff --git a/pkg/server/serverpb/status.proto b/pkg/server/serverpb/status.proto index a85c09072be1..cbeec3d0a58d 100644 --- a/pkg/server/serverpb/status.proto +++ b/pkg/server/serverpb/status.proto @@ -1484,11 +1484,11 @@ message HotRangesResponseV2 { double qps = 3 [ (gogoproto.customname) = "QPS" ]; - // table_name indicates the SQL table that the range belongs to. - string table_name = 4; - // database_name indicates on database that has current hot range. + // table_name has been deprecated in favor of tables = 16; + string table_name = 4 [deprecated = true]; + // database_name has been deprecated in favor of databases = 17; string database_name = 5; - // index_name indicates the index name for current range. + // index_name has been deprecated in favor of indexes = 17; string index_name = 6; // replica_node_ids specifies the list of node ids that contain replicas with current hot range. repeated int32 replica_node_ids = 7 [ @@ -1524,6 +1524,12 @@ message HotRangesResponseV2 { // CPU time (ns) per second is the recent cpu usage per second on this // range. double cpu_time_per_second = 15 [(gogoproto.customname) = "CPUTimePerSecond"]; + // Databases for the range. + repeated string databases = 16; + // Tables for the range + repeated string tables = 17; + // Indexes for the range + repeated string indexes = 18; } // Ranges contain list of hot ranges info that has highest number of QPS. repeated HotRange ranges = 1; diff --git a/pkg/server/status.go b/pkg/server/status.go index d350b62b9651..cdf3d247213e 100644 --- a/pkg/server/status.go +++ b/pkg/server/status.go @@ -46,6 +46,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/security" "github.com/cockroachdb/cockroach/pkg/security/username" "github.com/cockroachdb/cockroach/pkg/server/apiconstants" + "github.com/cockroachdb/cockroach/pkg/server/apiutil" "github.com/cockroachdb/cockroach/pkg/server/authserver" "github.com/cockroachdb/cockroach/pkg/server/debug" "github.com/cockroachdb/cockroach/pkg/server/diagnostics/diagnosticspb" @@ -57,8 +58,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/spanconfig" "github.com/cockroachdb/cockroach/pkg/sql" - "github.com/cockroachdb/cockroach/pkg/sql/catalog" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" "github.com/cockroachdb/cockroach/pkg/sql/clusterunique" "github.com/cockroachdb/cockroach/pkg/sql/contention" @@ -2829,13 +2828,6 @@ func (s *systemStatusServer) HotRanges( return response, nil } -type tableMeta struct { - dbName string - tableName string - schemaName string - indexName string -} - func (t *statusServer) HotRangesV2( ctx context.Context, req *serverpb.HotRangesRequest, ) (*serverpb.HotRangesResponseV2, error) { @@ -2878,8 +2870,6 @@ func (s *systemStatusServer) HotRangesV2( } } - var tableMetaCache syncutil.Map[roachpb.RangeID, tableMeta] - response := &serverpb.HotRangesResponseV2{ ErrorsByNodeID: make(map[roachpb.NodeID]string), } @@ -2893,70 +2883,30 @@ func (s *systemStatusServer) HotRangesV2( if local { resp := s.localHotRanges(ctx, tenantID) var ranges []*serverpb.HotRangesResponseV2_HotRange + var rangeIndexMappings map[roachpb.RangeID]apiutil.IndexNamesList for _, store := range resp.Stores { + rangeDescriptors := []roachpb.RangeDescriptor{} + for _, r := range store.HotRanges { + rangeDescriptors = append(rangeDescriptors, r.Desc) + } + if err = s.sqlServer.distSQLServer.DB.DescsTxn(ctx, func(ctx context.Context, txn descs.Txn) error { + databases, err := txn.Descriptors().GetAllDatabaseDescriptorsMap(ctx, txn.KV()) + if err != nil { + return err + } + rangeIndexMappings, err = apiutil.GetRangeIndexMapping(ctx, txn, s.sqlServer.execCfg.Codec, databases, rangeDescriptors) + return err + }); err != nil { + return nil, err + } for _, r := range store.HotRanges { - var ( - dbName, tableName, indexName, schemaName string - replicaNodeIDs []roachpb.NodeID - ) - rangeID := r.Desc.RangeID + var replicaNodeIDs []roachpb.NodeID + for _, repl := range r.Desc.Replicas().Descriptors() { replicaNodeIDs = append(replicaNodeIDs, repl.NodeID) } - if maybeIndexPrefix, tableID, ok := decodeTableID(s.sqlServer.execCfg.Codec, r.Desc.StartKey.AsRawKey()); !ok { - dbName = "system" - tableName = r.Desc.StartKey.String() - } else if meta, ok := tableMetaCache.Load(rangeID); ok { - dbName = meta.dbName - tableName = meta.tableName - schemaName = meta.schemaName - indexName = meta.indexName - } else { - if err = s.sqlServer.distSQLServer.DB.DescsTxn( - ctx, func(ctx context.Context, txn descs.Txn) error { - col := txn.Descriptors() - desc, err := col.ByIDWithoutLeased(txn.KV()).WithoutNonPublic().Get().Table(ctx, descpb.ID(tableID)) - if err != nil { - return errors.Wrapf(err, "cannot get table descriptor with tableID: %d, %s", tableID, r.Desc) - } - tableName = desc.GetName() - - if !maybeIndexPrefix.Equal(roachpb.KeyMin) { - if _, _, idxID, err := s.sqlServer.execCfg.Codec.DecodeIndexPrefix(r.Desc.StartKey.AsRawKey()); err != nil { - log.Warningf(ctx, "cannot decode index prefix for range descriptor: %s: %v", r.Desc, err) - } else { - if index := catalog.FindIndexByID(desc, descpb.IndexID(idxID)); index == nil { - log.Warningf(ctx, "cannot get index name for range descriptor: %s: index with ID %d not found", r.Desc, idxID) - } else { - indexName = index.GetName() - } - } - } - - if dbDesc, err := col.ByIDWithoutLeased(txn.KV()).WithoutNonPublic().Get().Database(ctx, desc.GetParentID()); err != nil { - log.Warningf(ctx, "cannot get database by descriptor ID: %s: %v", r.Desc, err) - } else { - dbName = dbDesc.GetName() - } - - if schemaDesc, err := col.ByIDWithoutLeased(txn.KV()).WithoutNonPublic().Get().Schema(ctx, desc.GetParentSchemaID()); err != nil { - log.Warningf(ctx, "cannot get schema name for range descriptor: %s: %v", r.Desc, err) - } else { - schemaName = schemaDesc.GetName() - } - return nil - }); err != nil { - log.Warningf(ctx, "failed to get table info for %s: %v", r.Desc, err) - continue - } - tableMetaCache.Store(rangeID, &tableMeta{ - dbName: dbName, - tableName: tableName, - schemaName: schemaName, - indexName: indexName, - }) - } + databases, tables, indexes := rangeIndexMappings[r.Desc.RangeID].ToOutput() ranges = append(ranges, &serverpb.HotRangesResponseV2_HotRange{ RangeID: r.Desc.RangeID, @@ -2967,13 +2917,12 @@ func (s *systemStatusServer) HotRangesV2( WriteBytesPerSecond: r.WriteBytesPerSecond, ReadBytesPerSecond: r.ReadBytesPerSecond, CPUTimePerSecond: r.CPUTimePerSecond, - TableName: tableName, - SchemaName: schemaName, - DatabaseName: dbName, - IndexName: indexName, ReplicaNodeIds: replicaNodeIDs, LeaseholderNodeID: r.LeaseholderNodeID, StoreID: store.StoreID, + Databases: databases, + Tables: tables, + Indexes: indexes, }) } } @@ -3018,23 +2967,6 @@ func (s *systemStatusServer) HotRangesV2( return response, nil } -func decodeTableID(codec keys.SQLCodec, key roachpb.Key) (roachpb.Key, uint32, bool) { - remaining, tableID, err := codec.DecodeTablePrefix(key) - if err != nil { - return nil, 0, false - } - // Validate that tableID doesn't belong to system or pseudo table. - if key.Equal(roachpb.KeyMin) || - tableID <= keys.SystemDatabaseID || - keys.IsPseudoTableID(tableID) || - bytes.HasPrefix(key, keys.Meta1Prefix) || - bytes.HasPrefix(key, keys.Meta2Prefix) || - bytes.HasPrefix(key, keys.SystemPrefix) { - return nil, 0, false - } - return remaining, tableID, true -} - func (s *systemStatusServer) localHotRanges( ctx context.Context, tenantID roachpb.TenantID, ) serverpb.HotRangesResponse_NodeResponse { diff --git a/pkg/sql/catalog/descs/collection.go b/pkg/sql/catalog/descs/collection.go index 61fabafc93ee..48fb9a6ceae9 100644 --- a/pkg/sql/catalog/descs/collection.go +++ b/pkg/sql/catalog/descs/collection.go @@ -1132,10 +1132,12 @@ func (tc *Collection) GetAllDatabaseDescriptors( return ret, nil } -// GetAllDatabaseDescriptorsMap returns the results of GetAllDatabaseDescriptors +// GetAllDatabaseDescriptorsMap returns the results of +// GetAllDatabaseDescriptors but as a map with the database ID as the +// key. func (tc *Collection) GetAllDatabaseDescriptorsMap( ctx context.Context, txn *kv.Txn, -) (ret map[descpb.ID]catalog.DatabaseDescriptor, _ error) { +) (map[descpb.ID]catalog.DatabaseDescriptor, error) { descriptors, err := tc.GetAllDatabaseDescriptors(ctx, txn) result := map[descpb.ID]catalog.DatabaseDescriptor{} if err != nil { @@ -1149,7 +1151,9 @@ func (tc *Collection) GetAllDatabaseDescriptorsMap( return result, nil } -// but as a map with the database ID as the key. +// GetSchemasForDatabase returns the schemas for a given database +// visible by the transaction. +// Deprecated: prefer GetAllSchemasInDatabase. func (tc *Collection) GetSchemasForDatabase( ctx context.Context, txn *kv.Txn, db catalog.DatabaseDescriptor, ) (map[descpb.ID]string, error) { diff --git a/pkg/sql/catalog/internal/catkv/catalog_reader.go b/pkg/sql/catalog/internal/catkv/catalog_reader.go index ed316ce5b823..99397d6fbcf5 100644 --- a/pkg/sql/catalog/internal/catkv/catalog_reader.go +++ b/pkg/sql/catalog/internal/catkv/catalog_reader.go @@ -209,13 +209,16 @@ func (cr catalogReader) ScanDescriptorsInSpans( ) (nstree.Catalog, error) { var mc nstree.MutableCatalog - descSpans := make([]roachpb.Span, len(spans)) - for i, span := range spans { + descSpans := []roachpb.Span{} + for _, span := range spans { descSpan, err := getDescriptorSpanFromSpan(cr.Codec(), span) if err != nil { return mc.Catalog, err } - descSpans[i] = descSpan + if descSpan.ZeroLength() { + continue + } + descSpans = append(descSpans, descSpan) } cq := catalogQuery{codec: cr.codec} diff --git a/pkg/ui/workspaces/db-console/src/views/hotRanges/hotRangesTable.tsx b/pkg/ui/workspaces/db-console/src/views/hotRanges/hotRangesTable.tsx index c7f94b57e40b..8b3c002cf304 100644 --- a/pkg/ui/workspaces/db-console/src/views/hotRanges/hotRangesTable.tsx +++ b/pkg/ui/workspaces/db-console/src/views/hotRanges/hotRangesTable.tsx @@ -231,8 +231,8 @@ const HotRangesTable = ({ Database ), - cell: val => <>{val.database_name}, - sort: val => val.database_name, + cell: val => <>{val.databases.join(", ")}, + sort: val => val.databases.join(", "), }, { name: "table", @@ -244,22 +244,7 @@ const HotRangesTable = ({ Table ), - cell: val => - // A hot range may not necessarily back a SQL table. If we see a - // "table name" that starts with a slash, it is not a table name but - // instead the start key of the range, and we should not link it. - val.table_name.startsWith("/") ? ( - val.table_name - ) : ( - - {val.table_name} - - ), + cell: val => val.tables.join(", "), sort: val => val.table_name, }, { @@ -272,8 +257,8 @@ const HotRangesTable = ({ Index ), - cell: val => <>{val.index_name}, - sort: val => val.index_name, + cell: val => <>{val.indexes.join(", ")}, + sort: val => val.indexes.join(", "), }, { name: "locality",