sql: report contention on writes in EXPLAIN ANALYZE #106354
Conversation
|
This change is |
yuzefovich
force-pushed
the
write-contention
branch
from
faf2bd6 to
0503940
Compare
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @cucaroach)
There was a problem hiding this comment.
Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status:
-- commits line 10 at r1:
typo: s/ran/run
pkg/sql/opt/exec/explain/output_test.go line 290 at r1 (raw file):
closedSem = true // Allow the main goroutine's mutation to experience contention. time.Sleep(3 * time.Second)
nit: Instead of a sleep, I think it would be more reliable to receive something sent from the main goroutine on a channel. Maybe it could re-use sem? Or use another channel?
yuzefovich
force-pushed
the
write-contention
branch
from
0503940 to
d1a30f1
Compare
This commit adds the contention events listener to `planNodeToRowSource` which allows us to add contention time information for mutation planNodes to be shown in EXPLAIN ANALYZE. Release note (sql change): CockroachDB now reports contention time encountered while executing mutation statements (INSERT, UPSERT, UPDATE, DELETE) when run via EXPLAIN ANALYZE.
yuzefovich
force-pushed
the
write-contention
branch
from
d1a30f1 to
628a1a7
Compare
There was a problem hiding this comment.
TFTRs!
bors r+
Reviewable status:
pkg/sql/opt/exec/explain/output_test.go line 290 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
nit: Instead of a sleep, I think it would be more reliable to receive something sent from the main goroutine on a channel. Maybe it could re-use
sem? Or use another channel?
It's a bit tricky since EXPLAIN ANALYZE UPSERT query will be blocked by the open txn, so we cannot do both things (execute the contending mutation and send on the channel) from the main goroutine, so I added another worker goroutine into the mix. The sleep is still present, but only to ensure non-trivial contention time (i.e. give the contending mutation a chance to get to the KV layer in its lifecycle).
There was a problem hiding this comment.
Reviewable status:
pkg/sql/opt/exec/explain/output_test.go line 290 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
It's a bit tricky since
EXPLAIN ANALYZE UPSERTquery will be blocked by the open txn, so we cannot do both things (execute the contending mutation and send on the channel) from the main goroutine, so I added another worker goroutine into the mix. The sleep is still present, but only to ensure non-trivial contention time (i.e. give the contending mutation a chance to get to the KV layer in its lifecycle).
Ah, right! Thanks.
|
Build succeeded: |
This commit adds the contention events listener to
planNodeToRowSourcewhich allows us to add contention time information for mutation planNodes to be shown in EXPLAIN ANALYZE.Fixes: #106266.
Release note (sql change): CockroachDB now reports contention time encountered while executing mutation statements (INSERT, UPSERT, UPDATE, DELETE) when run via EXPLAIN ANALYZE.