sql: add virtual indexes to crdb_internal.cluster_locks virtual table #79623
sql: add virtual indexes to crdb_internal.cluster_locks virtual table #79623craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
205066e to
c664630
Compare
c664630 to
6ab8db0
Compare
nvb
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @AlexTalks and @rafiss)
pkg/sql/crdb_internal.go, line 5837 at r3 (raw file):
contended BOOL, duration INTERVAL, INDEX(table_id),
Do we have tests that exercise each of these indexes?
pkg/sql/crdb_internal.go, line 5844 at r3 (raw file):
indexes: []virtualIndex{ { populate: func(
Could you give each of these objects that somehow indicate which INDEX they're implementing?
pkg/sql/crdb_internal.go, line 6023 at r3 (raw file):
IncludeUncontended: true, } if filters.contended != nil && *filters.contended {
If !*filters.contended, don't we want to include only uncontended locks?
4504d7f to
6b66286
Compare
AlexTalks
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/crdb_internal.go line 5837 at r3 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Do we have tests that exercise each of these indexes?
OK - Added these! I'm not sure if I should also add a small test that double checks the query plan to make sure it's using the index? If so let me know!
pkg/sql/crdb_internal.go line 5844 at r3 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Could you give each of these objects that somehow indicate which INDEX they're implementing?
Not sure I understand - you mean comments? Or you mean rename unwrappedConstraint to, say, tableNameConstraint for example?
pkg/sql/crdb_internal.go line 6023 at r3 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
If
!*filters.contended, don't we want to include only uncontended locks?
Yes - filtering this out, however, is already handled by the virtual schema table - only the rows that match the index constraint will be returned anyway, so we can instead focus here on limiting our request.
bdf1923 to
e46eafe
Compare
e46eafe to
12d37b0
Compare
12d37b0 to
a5e3892
Compare
AlexTalks
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/logictest/testdata/logic_test/cluster_locks line 149 at r5 (raw file):
Previously, otan (Oliver Tan) wrote…
can you add a test for
WHERE table_id = ...andWHERE table_name = ...?to get the tableid, you can probably do
WHERE table_id = 'table_name'::regclass::oid::int.
OK - had a few with table_name = ... previously, but I just added one with table_id.
AlexTalks
left a comment
There was a problem hiding this comment.
Dismissed @otan from a discussion.
Reviewable status:
AlexTalks
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/crdb_internal.go line 5844 at r3 (raw file):
Previously, AlexTalks (Alex Sarkesian) wrote…
Not sure I understand - you mean comments? Or you mean rename
unwrappedConstraintto, say,tableNameConstraintfor example?
Done. I refactored these a bit to use a generator, with the index-specific bits of logic passed in so it should now be pretty clear what these are.
This change adds virtual indexes on the `table_id`, `database_name`, and `table_name` columns of the `crdb_internal.cluster_locks` virtual table, so that when queried, the `kv.Batch`es with `QueryLocksRequest`s can be constrained to query only specific tables or databases. This allows the requests to be much more limited, rather than needing to request all of the ranges that comprise the key spans of all tables accessible by the user. Release note (sql change): Improved query performance for `crdb_internal.cluster_locks` when issued with constraints in the WHERE clause on `table_id`, `database_name`, or `table_name` columns.
a5e3892 to
f8a75ac
Compare
|
bors r+ |
|
Build succeeded: |
This change adds virtual indexes on the
table_id,database_name, andtable_namecolumns of thecrdb_internal.cluster_locksvirtual table,so that when queried, the
kv.Batches withQueryLocksRequests can beconstrained to query only specific tables or databases. This allows the
requests to be much more limited, rather than needing to request all of
the ranges that comprise the key spans of all tables accessible by the
user.
Release note (sql change): Improved query performance for
crdb_internal.cluster_lockswhen issued with constraints in the WHEREclause on
table_id,database_name, ortable_namecolumns.Depends on #77876, #80422