Skip to content

Commit

Permalink
sql: fix tracing of postqueries
Browse files Browse the repository at this point in the history
When we're performing tracing, we derive a special `consumeCtx` when
creating a DistSQLReceiver for the whole plan. That context should be
used for all components of the physical plan (main query, sub- and
post-queries), and the span needs to be finished by calling the stored
`cleanup` function. Previously, this was called in `ProducerDone` of the
main query which resulted in the span being finished before the
post-queries are run. As a result, the tracing spans for cascades and
checks could be incomplete.

This commit fixes the problem by delaying the finish of the span until
the DistSQLReceiver is released (since it is a convenient place that all
callers of `MakeDistSQLReceiver` call in a deferred invocation).

Release justification: bug fix.

Release note (bug fix): Previously, the traces of cascades and checks
could be incomplete, and now it is fixed.
  • Loading branch information
yuzefovich committed Mar 2, 2021
1 parent 0fe3d67 commit cb3e0aa
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 2 deletions.
6 changes: 4 additions & 2 deletions pkg/sql/distsql_running.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,9 @@ type DistSQLReceiver struct {

rangeCache *rangecache.RangeCache
tracing *SessionTracing
cleanup func()
// cleanup will be called when the DistSQLReceiver is Release()'d back to
// its sync.Pool.
cleanup func()

// The transaction in which the flow producing data for this
// receiver runs. The DistSQLReceiver updates the transaction in
Expand Down Expand Up @@ -572,6 +574,7 @@ func MakeDistSQLReceiver(

// Release releases this DistSQLReceiver back to the pool.
func (r *DistSQLReceiver) Release() {
r.cleanup()
*r = DistSQLReceiver{}
receiverSyncPool.Put(r)
}
Expand Down Expand Up @@ -782,7 +785,6 @@ func (r *DistSQLReceiver) ProducerDone() {
panic("double close")
}
r.closed = true
r.cleanup()
}

// Types is part of the RowReceiver interface.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,7 @@ WHERE message LIKE '%r$rangeid: sending batch%'
----
dist sender send r36: sending batch 1 DelRng to (n1,s1):1
dist sender send r36: sending batch 2 CPut to (n1,s1):1
dist sender send r36: sending batch 2 Scan to (n1,s1):1
dist sender send r36: sending batch 1 EndTxn to (n1,s1):1

query B
Expand All @@ -692,6 +693,7 @@ WHERE message LIKE '%r$rangeid: sending batch%'
dist sender send r36: sending batch 1 DelRng to (n1,s1):1
dist sender send r36: sending batch 1 Scan to (n1,s1):1
dist sender send r36: sending batch 1 Put to (n1,s1):1
dist sender send r36: sending batch 1 Scan to (n1,s1):1
dist sender send r36: sending batch 1 EndTxn to (n1,s1):1

query B
Expand All @@ -717,6 +719,7 @@ WHERE message LIKE '%r$rangeid: sending batch%'
dist sender send r36: sending batch 1 DelRng to (n1,s1):1
dist sender send r36: sending batch 1 Scan to (n1,s1):1
dist sender send r36: sending batch 1 Del to (n1,s1):1
dist sender send r36: sending batch 1 Scan to (n1,s1):1
dist sender send r36: sending batch 1 EndTxn to (n1,s1):1

# Test with a single cascade, which should use autocommit.
Expand All @@ -743,6 +746,7 @@ WHERE message LIKE '%r$rangeid: sending batch%'
----
dist sender send r36: sending batch 1 DelRng to (n1,s1):1
dist sender send r36: sending batch 1 DelRng to (n1,s1):1
dist sender send r36: sending batch 1 Scan to (n1,s1):1
dist sender send r36: sending batch 1 Del, 1 EndTxn to (n1,s1):1

# -----------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,7 @@ query T kvtrace
DELETE FROM t57085_p WHERE p = 1;
----
DelRange /Table/56/1/1 - /Table/56/1/1/#
Scan /Table/57/{1-2}
Del /Table/57/2/1/10/0
Del /Table/57/1/10/0
Del /Table/57/1/20/0

0 comments on commit cb3e0aa

Please sign in to comment.