kv: print traced query when failing the TestProxyTracing #135846
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @iskettaneh)
-- commits line 2 at r1:
nit: kvclient or kvcoord
pkg/kv/kvclient/kvcoord/dist_sender_server_test.go line 4956 at r1 (raw file):
).Scan(&msg, &tag, &loc); err != nil { // If we fail for any reason, print the trace to help debugging. printTrace()
Why do another SHOW TRACE FOR SESSION above if we're already doing it here?
Could we just do the t.Logf() here, given we have msg, tag, loc scanned already? Or, as part of the Fatalf string?
This commit prints the traced query when TestProxyTracing test fails. This should help with debugging the failures. References: cockroachdb#135493 Release note: None
iskettaneh
force-pushed
the
DeflakeProxy
branch
from
28551e3 to
7194126
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvclient/kvcoord/dist_sender_server_test.go line 4956 at r1 (raw file):
Previously, arulajmani (Arul Ajmani) wrote…
Why do another
SHOW TRACE FOR SESSIONabove if we're already doing it here?Could we just do the
t.Logf()here, given we have msg, tag, loc scanned already? Or, as part of theFatalfstring?
I added another one because the one that already exists has some filters that limit the query to one row only. But what I really wanted was to print all the rows in the traced statement. Therefore, next time it fails, we should hopefully be able to understand what happend exactly.
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status:
pkg/kv/kvclient/kvcoord/dist_sender_server_test.go line 4956 at r1 (raw file):
Previously, iskettaneh wrote…
I added another one because the one that already exists has some filters that limit the query to one row only. But what I really wanted was to print all the rows in the traced statement. Therefore, next time it fails, we should hopefully be able to understand what happend exactly.
Ack, I didn't look closely enough -- the query is different. Sounds good.
Separately, you may want to look at TestSecondaryTenantFollowerReadsRouting for an example of how we could write this test to not do this SQL parsing, and instead use a testing knob to trace. Given we're trying to print the trace here, I wonder if using testing knobs like TestSecondaryTenantFollowerReadsRouting would end up being cleaner.
There was a problem hiding this comment.
The comment about reworking the test is a nit. I don't want to block getting us a trace on the next failure, so feel free to address that in a separate PR (if you agree).
Reviewable status:
There was a problem hiding this comment.
ack
Reviewable status:
pkg/kv/kvclient/kvcoord/dist_sender_server_test.go line 4956 at r1 (raw file):
Previously, arulajmani (Arul Ajmani) wrote…
Ack, I didn't look closely enough -- the query is different. Sounds good.
Separately, you may want to look at
TestSecondaryTenantFollowerReadsRoutingfor an example of how we could write this test to not do this SQL parsing, and instead use a testing knob to trace. Given we're trying to print the trace here, I wonder if using testing knobs likeTestSecondaryTenantFollowerReadsRoutingwould end up being cleaner.
ack
|
TFTR bors r+ |
This commit prints the traced query when TestProxyTracing test fails. This should help with debugging the failures.
References: #135493
Release note: None