-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
scheduledlogging: TestCaptureIndexUsageStats is unreasonably long to run #87772
Labels
A-sql-logging-and-telemetry
Issues related to slow query log, SQL audit log, SQL internal logging telemetry, etc.
A-testing
Testing tools and infrastructure
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Comments
knz
added
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
A-testing
Testing tools and infrastructure
A-sql-logging-and-telemetry
Issues related to slow query log, SQL audit log, SQL internal logging telemetry, etc.
labels
Sep 10, 2022
knz
added a commit
to knz/cockroach
that referenced
this issue
Sep 11, 2022
This test takes between 1 and 2 minutes to run. (cockroachdb#87772) Release note: None
craig bot
pushed a commit
that referenced
this issue
Sep 12, 2022
87350: xform: avoid locality-optimized scans which must always read remote rows r=msirek a=msirek Previously, for tables with old-style partitioning, which don't use the new multiregion abstractions, there were no guardrails in place to prevent 2 cases where locality-optimized scan must always read ranges in remote regions: 1. When a scan with no hard limit has a non-unique index constraint (could return more than one row per matched index key, not including the partitioning key column) 2. When the max cardinality of a constrained scan is less than the hard limit placed on the scan via a LIMIT clause This was inadequate because locality-optimized scan is usually slower than distributed scans when reading from remote regions is required. If we can statically determine reading from remote regions is required, locality-optimized scan should not even be costed and considered by the optimizer. Multiregion tables, such as REGIONAL BY ROW tables, don't encounter this issue because the internal `crdb_region` partitioning column is not part of the UNIQUE constraint in that case, for example: ``` CREATE TABLE regional_by_row_table ( col1 int, col2 bool NOT NULL, UNIQUE INDEX idx(col1) -- crdb_region is implicitly the 1st idx col ) LOCALITY REGIONAL BY ROW; SELECT * FROM regional_by_row_table WHERE col1 = 1; ``` In the above, we could use LOS and split this into a local scan: `SELECT * FROM regional_by_row_table WHERE crdb_region = 'us' AND col1 = 1;` ... and remote scans: ``` SELECT * FROM regional_by_row_table WHERE crdb_region IN ('ca', 'ap') AND col1 = 1; ``` The max cardinality of the local scan is 1, and the max cardinality of the original scan is 1, so we know it's possible to fulfill the request solely with the local scan. To address this, this patch avoids planning locality-optimized scan for the two cases listed at the top of the description. The first case is detected by the local scan of the UNION ALL having a lower max cardinality than the max cardinality including all constraint spans (for example, given a pair of columns (part_col, col1), if col1 is a unique key, then max_cardinality(col1) will equal max_cardinality(part_col, col1). The second case is detected by a direct comparison of the hard limit with the max cardinality of the local scan. Release note (bug fix): This patch fixes a misused query optimization involving tables with one or more PARTITION BY clauses and partition zone constraints which assign region locality to those partitions. In some cases the optimizer picks a `locality-optimized search` query plan which is not truly locality-optimized, and has higher latency than competing query plans which use distributed scan. Locality-optimized search is now avoided in cases which are known not to benefit from this optimization. Release justification: Low risk fix for suboptimal locality-optimized scan 87773: scheduledlogging: various fixes to index stat collection r=xinhaoz a=knz As I was reviewing #87525, I noticed that the actual test cases were missing. Then, as I tried to fix that, I discovered a couple of other shortcomings. This PR fixes them. Fixes #87771. Informs #87772. 87803: ui: show app information on SQL Activity r=maryliag a=maryliag Add Application Name to: - Statement Overview <img width="670" alt="Screen Shot 2022-09-11 at 10 24 26 PM" src="https://user-images.githubusercontent.com/1017486/189563090-f60eeb52-07ba-47fd-8560-223ccfa091c4.png"> - Transaction Overview <img width="699" alt="Screen Shot 2022-09-11 at 10 24 41 PM" src="https://user-images.githubusercontent.com/1017486/189563108-2fd0599c-410b-42d6-affb-ef885908adab.png"> - Transaction Details <img width="1593" alt="Screen Shot 2022-09-11 at 10 24 52 PM" src="https://user-images.githubusercontent.com/1017486/189563118-ee1360cc-28a2-41cd-afbf-c0924b0c9fd8.png"> Fixes #87782 Partially Addresses #85229 Rename from "App" to "Application Name" on Statement Details. <img width="787" alt="Screen Shot 2022-09-11 at 10 25 08 PM" src="https://user-images.githubusercontent.com/1017486/189563136-a9d041b3-a667-409f-aa94-766ec23dafdd.png"> Release note (ui change): Add Application Name to Statement overview, Transaction Overview (and their respective column selectors), Transaction Details. Update label from "App" to "Application Name" on Statement Details page. 87821: roachtest: increase slowness threshold in streamer subtest r=yuzefovich a=yuzefovich This commit bumps up the slowness threshold for the `streamer` subtest from 1.25 to 1.5 in order to eliminate rare flakes on Q1 (which doesn't use the streamer at all). 1.5 threshold should still be sufficient as a sanity check (that the streamer doesn't become extremely slow). Fixes: #87791. Release note: None 87827: sql/schemachanger/rel,testutils/lint/passes/errcmp: add nolint support, use r=ajwerner a=ajwerner This error check was very expensive and was showing up meaningfully in profiles. Release note: None Co-authored-by: Mark Sirek <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]> Co-authored-by: Marylia Gutierrez <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Andrew Werner <[email protected]>
knz
added a commit
to knz/cockroach
that referenced
this issue
Sep 30, 2022
This test takes between 1 and 2 minutes to run. (cockroachdb#87772) Release note: None
knz
added a commit
to knz/cockroach
that referenced
this issue
Oct 4, 2022
This test takes between 1 and 2 minutes to run. (cockroachdb#87772) Release note: None
This was referenced Oct 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-sql-logging-and-telemetry
Issues related to slow query log, SQL audit log, SQL internal logging telemetry, etc.
A-testing
Testing tools and infrastructure
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Describe the problem
This test goes through 4 iterations of waiting for 20 seconds each time for something to happen.
This is abnormally long for a unit test, and is not even necessary.
Expected behavior
The test can be modified to run under 10 seconds in total.
cc @maryliag @THardy98
Jira issue: CRDB-19525
The text was updated successfully, but these errors were encountered: