colexec: improve tracing in the vectorized engine #55821
Labels
C-cleanup
Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior.
T-sql-queries
SQL Queries Team
Comments
yuzefovich
added
the
C-cleanup
yuzefovich
added this to Triage
in BACKLOG, NO NEW ISSUES: SQL Execution
via automation
asubiotto
moved this from Triage
to [VECTORIZED BACKLOG] Enhancements/Features/Investigations
in BACKLOG, NO NEW ISSUES: SQL Execution
craig bot
pushed a commit
that referenced
this issue
Apr 23, 2021
61223: colexecop: change Operator interface r=yuzefovich a=yuzefovich This commit moves the context argument from `Next()` method into `Init()`. The idea is that now the `Operator` interface looks similar to `RowSource` with the following context management: - in `Init`, all operators are expected to capture the passed-in context. They could optionally derive their own context if necessary and pass that derived context into its inputs. The contract of `Init` method now is that all calls except for the first one are noops. - any work performed in `Next` that needs a context should use the one captured by the operator. The follow-up commit will remove the context argument from `DrainMeta` signature. The motivation for this change is that we want to improve tracing in the vectorized engine. In the row-by-row engine we create a tracing span for every processor which allows us to more precisely attribute the time spent during the execution to each of the processors. However, in the vectorized engine we currently only create spans at root components (either root materializer or outboxes) as well as in ColBatchScans (in order to disambiguate the contention events coming from different components). After this change we will be able to easily start tracing spans in `Init` of the operators. In order to simplify the handling of the context this commit introduces several utility structs into `colexecop` package. Many operators that don't need to perform any work in their `Init` method other than calling `Init` on its single input can now embed `OneInputHelper` and "inherit" the default implementation of `Init`. A notable change is to an Inbox - we now remove `flowCtx` which was captured temporarily until this change (of adding context argument to `Init`) is made. Addresses: #55821. Release note: None Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
|
We have marked this issue as stale because it has been inactive for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
All processors in the row-by-row engine have decent tracing in place (
ProcessorBase.StartInternalcreates a span for the processor inStartmethod and then when moving to trailing meta stateGetTraceDatacollects that information and propagates it as the metadata). We currently don't have anything like this for each operator, and it'd be nice to add that.Jira issue: CRDB-3626
The text was updated successfully, but these errors were encountered: