sql: reduce the overhead of EXPLAIN ANALYZE #91117
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @michae2)
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
In order to propagate the execution stats across the distributed query plan we use the tracing infrastructure, where each stats object is added as "structured metadata" to the trace. Thus, whenever we're collecting the exec stats for a statement, we must enable tracing. Previously, in many cases we would enable it at the highest verbosity level which has non-trivial overhead. In some cases this was an overkill (e.g. in `EXPLAIN ANALYZE` we don't really care about the trace containing all of the gory details - we won't expose it anyway), so this is now fixed by using the less verbose "structured" verbosity level. As a concrete example of the difference: for a stmt that without `EXPLAIN ANALYZE` takes around 190ms, with `EXPLAIN ANALYZE` it would previously run for about 1.8s and now it takes around 210ms. This required some minor changes to the row-by-row outbox and router setups to collect thats even if the recording is not verbose. Release note (performance improvement): The overhead of running `EXPLAIN ANALYZE` and `EXPLAIN ANALYZE (DISTSQL)` has been significantly reduced. The overhead of `EXPLAIN ANALYZE (DEBUG)` didn't change.
yuzefovich
force-pushed
the
fix-explain-analyze
branch
from
9e6f18c
to
98ee45d
Compare
|
I needed to make some minor tweaks. I wonder though - should this PR completely close #90739? It seems like |
The original slack thread included a statement bundle. I wrote |
There was a problem hiding this comment.
Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status:
|
TFTRs! bors r+ |
|
This PR was included in a batch that timed out, it will be automatically retried |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 98ee45d to blathers/backport-release-22.1-91117: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. error creating merge commit from 98ee45d to blathers/backport-release-22.2-91117: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
In order to propagate the execution stats across the distributed query plan we use the tracing infrastructure, where each stats object is added as "structured metadata" to the trace. Thus, whenever we're collecting the exec stats for a statement, we must enable tracing. Previously, in many cases we would enable it at the highest verbosity level which has non-trivial overhead. In some cases this was an overkill (e.g. in
EXPLAIN ANALYZEwe don't really care about the trace containing all of the gory details - we won't expose it anyway), so this is now fixed by using the less verbose "structured" verbosity level. As a concrete example of the difference: for a stmt that withoutEXPLAIN ANALYZEtakes around 190ms, withEXPLAIN ANALYZEit would previously run for about 1.8s and now it takes around 210ms.This required some minor changes to the row-by-row outbox and router
setups to collect thats even if the recording is not verbose.
Addresses: #90739.
Epic: CRDB-14510
Release note (performance improvement): The overhead of running
EXPLAIN ANALYZEandEXPLAIN ANALYZE (DISTSQL)has been significantly reduced. The overhead ofEXPLAIN ANALYZE (DEBUG)didn't change.