-
Notifications
You must be signed in to change notification settings - Fork 4.1k
sql: remove some EXPLAIN [ANALYZE] fields in non-VERBOSE mode #153361
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
Conversation
95b820e to
4cf75a1
Compare
yuzefovich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I only have one consideration.
@yuzefovich reviewed 4 of 4 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @kyle-a-wong)
-- commits line 16 at r2:
I wonder whether we should show it if we have vectorized: false?
DrewKimball
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DrewKimball reviewed 4 of 4 files at r1, 6 of 6 files at r2, all commit messages.
Reviewable status:complete! 2 of 0 LGTMs obtained (waiting on @kyle-a-wong and @mgartner)
2b2f3fd to
c9b6e40
Compare
Previously, yuzefovich (Yahor Yuzefovich) wrote…
Sure, done. |
e67fac4 to
8a7fb14
Compare
|
Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
8a7fb14 to
45d9231
Compare
45d9231 to
f1ba18f
Compare
a5eb552 to
5cec95c
Compare
5cec95c to
0763175
Compare
The output of `EXPLAIN [ANALYZE]` in non-`VERBOSE` mode is now more succinct. In the header above the query plan the following changes have been made: 1. "isolation level" is only shown if it is not "serializable". 2. "priority" is only shown if it is not "normal". 3. "quality of service" is only shown if it is not "regular". 4. "kv cpu time" is not shown. 5. "sql cpu time" is not shown. 6. "DistSQL network usage" is only show if it is greater than 0. 7. "maximum memory usage" is not shown. 8. "cumulative time spent in KV" is not shown. 9. "vectorized" is only shown if it is "false". 10. "buffered writes enabled" is not shown. In each node of the query plan the following changes have been made: 1. "KV pairs read" is not shown. 2. "KV bytes read" is not shown. 3. "KV gRPC calls" is not shown. 4. "estimated max memory allocated" is not shown. 5. "estimated max sql temp disk usage" is not shown. 6. "actual row count" is directly above "estimated row count". Here's an example of what `EXPLAIN ANALYZE` looked like before: ``` planning time: 243µs execution time: 632µs distribution: local vectorized: true plan type: generic, reused cumulative time spent in KV: 331µs maximum memory usage: 20 KiB DistSQL network usage: 0 B (0 messages) regions: us-east1 sql cpu time: 20µs estimated RUs consumed: 0 isolation level: serializable priority: normal quality of service: regular • scan sql nodes: n1 kv nodes: n1 regions: us-east1 actual row count: 0 KV time: 331µs KV rows decoded: 0 KV bytes read: 0 B KV gRPC calls: 1 estimated max memory allocated: 20 KiB sql cpu time: 20µs missing stats table: t@t_pkey spans: [/0 - /0] ``` And here's the output with the same query after the changes in this commit: ``` planning time: 325µs execution time: 974µs distribution: local plan type: generic, reused regions: us-east1 estimated RUs consumed: 0 • scan sql nodes: n1 kv nodes: n1 regions: us-east1 KV time: 610µs KV rows decoded: 0 sql cpu time: 29µs actual row count: 0 missing stats table: t@t_pkey spans: [/0 - /0] ``` Release note (sql change): The output of `EXPLAIN [ANALYZE]` in non-`VERBOSE` mode is now more succinct.
This test file relies on awaiting for the rangefeed to catch up, so let's reduce the closed TS interval. Release note: None
0763175 to
5d3ad6b
Compare
yuzefovich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed up a few remaining and new spots.
bors r=yuzefovich,drewkimball
@yuzefovich reviewed 135 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status:complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @mgartner).
|
bors r+ |
153361: sql: remove some EXPLAIN [ANALYZE] fields in non-VERBOSE mode r=yuzefovich a=mgartner The output of `EXPLAIN [ANALYZE]` in non-`VERBOSE` mode is now more succinct. In the header above the query plan the following changes have been made: 1. "isolation level" is only shown if it is not "serializable". 2. "priority" is only shown if it is not "normal". 3. "quality of service" is only shown if it is not "regular". 4. "kv cpu time" is not shown. 5. "sql cpu time" is not shown. 6. "DistSQL network usage" is only show if it is greater than 0. 7. "maximum memory usage" is not shown. 8. "cumulative time spent in KV" is not shown. 9. "vectorized" is only shown if it is "false". 10. "buffered writes enabled" is not shown. In each node of the query plan the following changes have been made: 1. "KV pairs read" is not shown. 2. "KV bytes read" is not shown. 3. "KV gRPC calls" is not shown. 4. "estimated max memory allocated" is not shown. 5. "estimated max sql temp disk usage" is not shown. 6. "actual row count" is directly above "estimated row count". Here's an example of what `EXPLAIN ANALYZE` looked like before: ``` planning time: 243µs execution time: 632µs distribution: local vectorized: true plan type: generic, reused cumulative time spent in KV: 331µs maximum memory usage: 20 KiB DistSQL network usage: 0 B (0 messages) regions: us-east1 sql cpu time: 20µs estimated RUs consumed: 0 isolation level: serializable priority: normal quality of service: regular • scan sql nodes: n1 kv nodes: n1 regions: us-east1 actual row count: 0 KV time: 331µs KV rows decoded: 0 KV bytes read: 0 B KV gRPC calls: 1 estimated max memory allocated: 20 KiB sql cpu time: 20µs missing stats table: t@t_pkey spans: [/0 - /0] ``` And here's the output with the same query after the changes in this commit: ``` planning time: 325µs execution time: 974µs distribution: local plan type: generic, reused regions: us-east1 estimated RUs consumed: 0 • scan sql nodes: n1 kv nodes: n1 regions: us-east1 KV time: 610µs KV rows decoded: 0 sql cpu time: 29µs actual row count: 0 missing stats table: t@t_pkey spans: [/0 - /0] ``` Epic: None Release note (sql change): The output of `EXPLAIN [ANALYZE]` in non-`VERBOSE` mode is now more succinct. 160206: sql: clean up dependency graph a bit r=yuzefovich a=yuzefovich This PR contains a few commits that break a few dependencies to allow more parallelism during builds. Informs: #79357. Epic: None Release note: None 160557: sctest: unskip multitenant for schema change tests r=rafiss a=rafiss This fixes external process tenant startup to propagate version settings from the parent, allowing tenants to start when the cluster is running at an older version (e.g., MinSupported). We also had to pass in the SQLCodec so that the schema TestDeps accesss the tenant correctly. Note that MultiRegionTestClusterFactory already allowed multitenant testing. fixes #142814 fixes #109457 Release note: None Co-authored-by: Marcus Gartner <[email protected]> Co-authored-by: Yahor Yuzefovich <[email protected]> Co-authored-by: Rafi Shamim <[email protected]>
|
Build failed (retrying...): |
The output of
EXPLAIN [ANALYZE]in non-VERBOSEmode is now more succinct.In the header above the query plan the following changes have been made:
In each node of the query plan the following changes have been made:
Here's an example of what
EXPLAIN ANALYZElooked like before:And here's the output with the same query after the changes in this commit:
Epic: None
Release note (sql change): The output of
EXPLAIN [ANALYZE]in non-VERBOSEmode is now more succinct.