Skip to content

sql: trace execution stats for query diagnostics#46132

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
RaduBerinde:diag-trace-ctx
Mar 17, 2020
Merged

sql: trace execution stats for query diagnostics#46132
craig[bot] merged 1 commit intocockroachdb:masterfrom
RaduBerinde:diag-trace-ctx

Conversation

@RaduBerinde
Copy link
Copy Markdown
Member

The distsql processors are initialized with the Context in the eval context. If
this context contains a tracing span that is recording, the processors will set
up statistics collection and put them in the span as tags.

The statement diagnostics code sets up a span but doesn't change this context,
so statistics collection doesn't happen. We want these statistics in the trace,
as they will soon be used to generate EXPLAIN ANALYZE diagrams for the bundles.

This change fixes this issue and moves up the initialization of the planner so
we can tweak it directly, which simplifies code.

Release note (bug fix): statement diagnostics traces now contain processor
statistics.

Release justification: Bug fixes and low-risk updates to new functionality

@RaduBerinde RaduBerinde requested a review from andreimatei March 15, 2020 16:25
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @andreimatei)

@jordanlewis
Copy link
Copy Markdown
Member

cc @asubiotto

@RaduBerinde
Copy link
Copy Markdown
Member Author

bors r+

@RaduBerinde
Copy link
Copy Markdown
Member Author

bors r-

Seems like it's still not working well for subqueries.

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 16, 2020

Canceled

The distsql processors are initialized with the Context in the eval context. If
this context contains a tracing span that is recording, the processors will set
up statistics collection and put them in the span as tags.

The statement diagnostics code sets up a span but doesn't change this context,
so statistics collection doesn't happen. We want these statistics in the trace,
as they will soon be used to generate EXPLAIN ANALYZE diagrams for the bundles.

This change fixes this issue and moves up the initialization of the planner so
we can tweak it directly, which simplifies code.

Release note (bug fix): statement diagnostics traces now contain processor
statistics.

Release justification: Bug fixes and low-risk updates to new functionality
@RaduBerinde
Copy link
Copy Markdown
Member Author

Added similar code for the Context used by sub/postqueries, PTAL.

@RaduBerinde
Copy link
Copy Markdown
Member Author

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 17, 2020

Build succeeded

@craig craig bot merged commit bbe20c3 into cockroachdb:master Mar 17, 2020
@RaduBerinde RaduBerinde deleted the diag-trace-ctx branch March 17, 2020 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants