Skip to content

Commit

Permalink
sql: TestRandomSyntaxFunctions skip schema telemetry job function
Browse files Browse the repository at this point in the history
Previously, this test could depending on the concurrency,
could spawn a large number of schema telemetry jobs via
(ctdb_internal.create_sql_schema_telemetry_job).
This could lead to contention that will eventually
cause this test to time out. To address this, this
patch limits calling the telemetry job creation function.

Fixes: cockroachdb#108153

Release note: None
  • Loading branch information
fqazi committed Aug 18, 2023
1 parent 85ff639 commit f87ebd4
Showing 1 changed file with 8 additions and 1 deletion.
9 changes: 8 additions & 1 deletion pkg/sql/tests/rsg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ func (db *verifyFormatDB) execWithResettableTimeout(
}()
retry := true
targetDuration := duration
cancellationChannel := ctx.Done()
for retry {
retry = false
err := func() error {
Expand Down Expand Up @@ -202,7 +203,7 @@ func (db *verifyFormatDB) execWithResettableTimeout(
return &nonCrasher{sql: sql, err: err}
}
return nil
case <-ctx.Done():
case <-cancellationChannel:
// Sanity: The context is cancelled when the test is about to
// timeout. We will log whatever statement we're waiting on for
// debugging purposes. Sometimes queries won't respect
Expand All @@ -211,6 +212,7 @@ func (db *verifyFormatDB) execWithResettableTimeout(
// We will intentionally retry, which will us to wait for the
// go routine to complete above to avoid leaking it.
retry = true
cancellationChannel = nil
return nil
case <-time.After(targetDuration):
db.mu.Lock()
Expand Down Expand Up @@ -358,6 +360,10 @@ func TestRandomSyntaxFunctions(t *testing.T) {
switch lower {
case "pg_sleep":
continue
case "crdb_internal.create_sql_schema_telemetry_job":
// We can create a crazy number of telemtry jobs accidentally,
// within the test. Leading to terrible contention.
continue
case "crdb_internal.gen_rand_ident":
// Generates random identifiers, so a large number are dangerous and
// can take a long time.
Expand Down Expand Up @@ -437,6 +443,7 @@ func TestRandomSyntaxFunctions(t *testing.T) {
// involve schema changes like truncates. In general this should make
// this test more resilient as the timeouts are reset as long progress
// is made on *some* connection.
t.Logf("Running %q", s)
return db.execWithResettableTimeout(t, ctx, s, *flagRSGExecTimeout, *flagRSGGoRoutines)
})
}
Expand Down

0 comments on commit f87ebd4

Please sign in to comment.