roachtest: reduce hangs in acceptance-chaos tests #37205
Conversation
These tests are pretty janky, and can end up failing with a timeout and a deadlocked test, which is not something roachtest can really ever handle gracefully. Sprinkle more contexts around and set a statement timeout for the central query that is most likely to get stuck under the crucial lock that we think "causes" most of the deadlocks. Of course there is likely a real problem with CRDB, which this PR does nothing about. All that is (hopefully) achieved here is a clean failure mode. The failure prompting this PR is fixed by cockroachdb#37204, unfortunately it also turns out that the statement timeout added in this PR did not prevent the statement from hanging. It is probably still worth merging this. Release note: None
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @andreimatei and @tbg)
pkg/cmd/roachtest/bank.go, line 66 at r1 (raw file):
SET statement_timeout = '31s'; UPDATE bank.accounts SET balance = CASE id WHEN %[1]d THEN balance-%[3]d WHEN %[2]d THEN balance+%[3]d END
how come you moved to string interpolation here?
There was a problem hiding this comment.
acceptance/bank/cluster-recovery runs this code. This code has this lock around the clients slice. If queries don't return in a timely manner (particular the update txn), the test will deadlock because the RLock is never released.
TFTR!
bors r=andreimatei
Reviewable status:
pkg/cmd/roachtest/bank.go, line 66 at r1 (raw file):
I think you missed the comment in the code:
Run with a statement timeout, which unfortunately precludes the use of prepared statements.
37205: roachtest: reduce hangs in acceptance-chaos tests r=andreimatei a=tbg These tests are pretty janky, and can end up failing with a timeout and a deadlocked test, which is not something roachtest can really ever handle gracefully. Sprinkle more contexts around and set a statement timeout for the central query that is most likely to get stuck under the crucial lock that we think "causes" most of the deadlocks. Of course there is likely a real problem with CRDB, which this PR does nothing about. All that is (hopefully) achieved here is a clean failure mode. The failure prompting this PR is fixed by #37204, unfortunately it also turns out that the statement timeout added in this PR did not prevent the statement from hanging. It is probably still worth merging this. Release note: None Co-authored-by: Tobias Schottdorf <[email protected]>
Build succeeded |
These tests are pretty janky, and can end up failing with a timeout and
a deadlocked test, which is not something roachtest can really ever
handle gracefully. Sprinkle more contexts around and set a statement
timeout for the central query that is most likely to get stuck under the
crucial lock that we think "causes" most of the deadlocks.
Of course there is likely a real problem with CRDB, which this PR does
nothing about. All that is (hopefully) achieved here is a clean failure
mode. The failure prompting this PR is fixed by #37204, unfortunately
it also turns out that the statement timeout added in this PR did not
prevent the statement from hanging. It is probably still worth merging
this.
Release note: None