Skip to content

Commit cfc8f32

Browse files
committed
roachtest: avoid known problematic pattern in sqlsmith
We are aware of one query pattern where sqlsmith roachtest might time out (runtime assertions enabled AND one of the system columns that requires MVCC decoding is requested). This is unfortunate but expected behavior that we don't plan on improving on, so let's avoid generating this query pattern. Release note: None
1 parent b4b4d63 commit cfc8f32

File tree

1 file changed

+35
-8
lines changed

1 file changed

+35
-8
lines changed

pkg/cmd/roachtest/tests/sqlsmith.go

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/cluster"
1919
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/option"
2020
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/registry"
21+
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/roachtestutil"
2122
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/spec"
2223
"github.com/cockroachdb/cockroach/pkg/cmd/roachtest/test"
2324
"github.com/cockroachdb/cockroach/pkg/internal/sqlsmith"
@@ -184,22 +185,27 @@ WITH into_db = 'defaultdb';
184185
}
185186
}()
186187

187-
stmt = smither.Generate()
188-
if stmt == "" {
189-
// If an empty statement is generated, then ignore it.
190-
done <- errors.Newf("Empty statement returned by generate")
191-
return nil
188+
for {
189+
stmt = smither.Generate()
190+
if stmt == "" {
191+
// If an empty statement is generated, then ignore it.
192+
done <- errors.Newf("Empty statement returned by generate")
193+
return nil
194+
}
195+
if !expectedToTimeout(t, stmt) {
196+
break
197+
}
192198
}
193199

194200
// TODO(yuzefovich): investigate why using the context with
195201
// a timeout results in poisoning the connection (#101208).
196202
_, err := conn.Exec(stmt)
197203
if err == nil {
198204
logStmt(stmt)
199-
stmt = "EXPLAIN " + stmt
200-
_, err = conn.Exec(stmt)
205+
explainStmt := "EXPLAIN " + stmt
206+
_, err = conn.Exec(explainStmt)
201207
if err == nil {
202-
logStmt(stmt)
208+
logStmt(explainStmt)
203209
}
204210
}
205211
done <- err
@@ -329,6 +335,27 @@ WITH into_db = 'defaultdb';
329335
register("seed-multi-region", "multi-region")
330336
}
331337

338+
// expectedToTimeout returns whether the given stmt is likely to timeout due to
339+
// expected edge case behavior.
340+
func expectedToTimeout(t test.Test, stmt string) bool {
341+
// The only known scenario right now (see #157971) is when we have runtime
342+
// assertions enabled AND the query uses a system column that requires MVCC
343+
// decoding.
344+
if !roachtestutil.UsingRuntimeAssertions(t) {
345+
return false
346+
}
347+
for _, sysCol := range []string{
348+
"crdb_internal_mvcc_timestamp",
349+
"crdb_internal_origin_id",
350+
"crdb_internal_origin_timestamp",
351+
} {
352+
if strings.Contains(stmt, sysCol) {
353+
return true
354+
}
355+
}
356+
return false
357+
}
358+
332359
// setupMultiRegionDatabase is used to set up a multi-region database.
333360
func setupMultiRegionDatabase(t test.Test, conn *gosql.DB, rnd *rand.Rand, logStmt func(string)) {
334361
t.Helper()

0 commit comments

Comments
 (0)