From d65c8afe717274d2702c1ba69d88b8be73466c17 Mon Sep 17 00:00:00 2001 From: Mark Sirek Date: Mon, 23 Jan 2023 18:57:03 -0800 Subject: [PATCH] tests: fix fatal errors from timeout-out statements in queryComparisonTests Fixes #95680 Roachtests such as tlp, costfuzz or unoptimized-query-oracle may time out helper statements such as `SET testing_optimizer_random_seed = xxx` and misinterpret the error as a fatal error, causing an issue to be opened. This fixes the problem by not erroring out the test if the test or round timeout has expired. An alternative fix could have been to avoid throwing errors for SET statements, with the idea that such errors are likely rare. The current fix was chosen so that we still might catch such errors if they were to occur. Release note: None --- .../roachtest/tests/query_comparison_util.go | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/roachtest/tests/query_comparison_util.go b/pkg/cmd/roachtest/tests/query_comparison_util.go index 446f8b9c7e8f..beacf4313816 100644 --- a/pkg/cmd/roachtest/tests/query_comparison_util.go +++ b/pkg/cmd/roachtest/tests/query_comparison_util.go @@ -207,14 +207,21 @@ func runOneRoundQueryComparison( until := time.After(roundTimeout) done := ctx.Done() - for i := 1; ; i++ { + roundOrTestTimeout := func() bool { select { case <-until: - return + return true case <-done: - return + return true default: } + return false + } + + for i := 1; ; i++ { + if roundOrTestTimeout() { + return + } const numInitialMutations = 1000 @@ -248,7 +255,16 @@ func runOneRoundQueryComparison( stmtNo: i, } if err := qct.run(smither, rnd, h); err != nil { - t.Fatal(err) + // The error may just be due to the timeout occurring. Don't report the + // error if the test timeout or 10-minute round timeout has elapsed. There + // might be a small chance of missing actual query mismatches if one + // occurs within a millisecond or so of the timeout, but the likelihood of + // this is very small. It's better to have a stable test. If we really + // care about this, we could dig into the actual error and see if we can + // tell if it was due to a timeout or not. + if !roundOrTestTimeout() { + t.Fatal(err) + } } } }