Skip to content

Commit

Permalink
sql: fix tracking of query level stats for sub- and post-queries
Browse files Browse the repository at this point in the history
Previously, we were creating a new `topLevelQueryStats` object for each
sub- and post-query, and as a result we would accumulate the top level
query stats only for the main query. I think this is undesirable and is
now fixed.

Release note: None

Release justification: low-risk bug fix.
  • Loading branch information
yuzefovich committed Aug 26, 2021
1 parent 9844dee commit 139fd0c
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 8 deletions.
6 changes: 3 additions & 3 deletions pkg/sql/conn_executor_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -1272,7 +1272,7 @@ func (ex *connExecutor) execWithDistSQLEngine(
if !ex.server.cfg.DistSQLPlanner.PlanAndRunSubqueries(
ctx, planner, evalCtxFactory, planner.curPlan.subqueryPlans, recv, &subqueryResultMemAcc,
) {
return recv.stats, recv.commErr
return *recv.stats, recv.commErr
}
}
recv.discardRows = planner.instrumentation.ShouldDiscardRows()
Expand All @@ -1285,14 +1285,14 @@ func (ex *connExecutor) execWithDistSQLEngine(
// need to have access to the main query tree.
defer cleanup()
if recv.commErr != nil || res.Err() != nil {
return recv.stats, recv.commErr
return *recv.stats, recv.commErr
}

ex.server.cfg.DistSQLPlanner.PlanAndRunCascadesAndChecks(
ctx, planner, evalCtxFactory, &planner.curPlan.planComponents, recv,
)

return recv.stats, recv.commErr
return *recv.stats, recv.commErr
}

// beginTransactionTimestampsAndReadMode computes the timestamps and
Expand Down
8 changes: 3 additions & 5 deletions pkg/sql/distsql_running.go
Original file line number Diff line number Diff line change
Expand Up @@ -608,11 +608,7 @@ type DistSQLReceiver struct {
// this node's clock.
clockUpdater clockUpdater

// TODO(yuzefovich): I believe these stats currently only include the
// metrics from the main query, and not from any sub- or post-queries
// because we use DistSQLReceiver.clone() for those. Think through whether
// this is expected or not.
stats topLevelQueryStats
stats *topLevelQueryStats

expectedRowsRead int64
progressAtomic *uint64
Expand Down Expand Up @@ -757,6 +753,7 @@ func MakeDistSQLReceiver(
rangeCache: rangeCache,
txn: txn,
clockUpdater: clockUpdater,
stats: &topLevelQueryStats{},
stmtType: stmtType,
tracing: tracing,
contentionRegistry: contentionRegistry,
Expand All @@ -782,6 +779,7 @@ func (r *DistSQLReceiver) clone() *DistSQLReceiver {
rangeCache: r.rangeCache,
txn: r.txn,
clockUpdater: r.clockUpdater,
stats: r.stats,
stmtType: tree.Rows,
tracing: r.tracing,
contentionRegistry: r.contentionRegistry,
Expand Down

0 comments on commit 139fd0c

Please sign in to comment.