Merge pull request cockroachdb#104672 from cockroachdb/blathers/backp…

…ort-release-23.1-104549 release-23.1: sql/tests: deflake TestErrorDuringExtendedProtocolCommit and TestCancelQueriesRace
srosenberg · Jun 9, 2023 · 96866cf · 96866cf
2 parents c63ae6c + 9aad7d2
commit 96866cf
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 7 deletions.
diff --git a/pkg/sql/show_test.go b/pkg/sql/show_test.go
@@ -1201,12 +1201,14 @@ func TestCancelQueriesRace(t *testing.T) {
 		_, _ = sqlDB.ExecContext(ctx, `SELECT pg_sleep(10)`)
 		close(waiter)
 	}()
-	_, _ = sqlDB.ExecContext(ctx, `CANCEL QUERIES (
+	_, err := sqlDB.ExecContext(ctx, `CANCEL QUERIES (
 		SELECT query_id FROM [SHOW QUERIES] WHERE query LIKE 'SELECT pg_sleep%'
 	)`)
-	_, _ = sqlDB.ExecContext(ctx, `CANCEL QUERIES (
+	require.NoError(t, err)
+	_, err = sqlDB.ExecContext(ctx, `CANCEL QUERIES (
 		SELECT query_id FROM [SHOW QUERIES] WHERE query LIKE 'SELECT pg_sleep%'
 	)`)
+	require.NoError(t, err)
 
 	cancel()
 	<-waiter

diff --git a/pkg/sql/tests/BUILD.bazel b/pkg/sql/tests/BUILD.bazel
@@ -121,7 +121,6 @@ go_test(
         "//pkg/util/syncutil",
         "//pkg/util/timeutil",
         "//pkg/util/tracing",
-        "//pkg/util/tracing/tracingpb",
         "//pkg/util/uuid",
         "@com_github_cockroachdb_cockroach_go_v2//crdb",
         "@com_github_cockroachdb_cockroach_go_v2//crdb/crdbpgx",

diff --git a/pkg/sql/tests/autocommit_extended_protocol_test.go b/pkg/sql/tests/autocommit_extended_protocol_test.go
@@ -16,6 +16,7 @@ import (
 	"errors"
 	"net/url"
 	"strings"
+	"sync/atomic"
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
@@ -27,7 +28,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
-	"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
 	"github.com/jackc/pgx/v4"
 	"github.com/stretchr/testify/require"
 )
@@ -158,14 +158,14 @@ func TestErrorDuringExtendedProtocolCommit(t *testing.T) {
 	var db *gosql.DB
 
 	var shouldErrorOnAutoCommit syncutil.AtomicBool
-	var traceID tracingpb.TraceID
+	var traceID atomic.Uint64
 	params, _ := CreateTestServerParams()
 	params.Knobs.SQLExecutor = &sql.ExecutorTestingKnobs{
 		DisableAutoCommitDuringExec: true,
 		BeforeExecute: func(ctx context.Context, stmt string, descriptors *descs.Collection) {
 			if strings.Contains(stmt, "SELECT 'cat'") {
 				shouldErrorOnAutoCommit.Set(true)
-				traceID = tracing.SpanFromContext(ctx).TraceID()
+				traceID.Store(uint64(tracing.SpanFromContext(ctx).TraceID()))
 			}
 		},
 		BeforeAutoCommit: func(ctx context.Context, stmt string) error {
@@ -174,7 +174,7 @@ func TestErrorDuringExtendedProtocolCommit(t *testing.T) {
 				// saw when executing our test query. This is so we know that this
 				// autocommit corresponds to our test qyery rather than an internal
 				// query.
-				if traceID == tracing.SpanFromContext(ctx).TraceID() {
+				if traceID.Load() == uint64(tracing.SpanFromContext(ctx).TraceID()) {
 					shouldErrorOnAutoCommit.Set(false)
 					return errors.New("injected error")
 				}

diff --git a/pkg/util/leaktest/leaktest.go b/pkg/util/leaktest/leaktest.go
@@ -62,6 +62,8 @@ func interestingGoroutines() map[int64]string {
 			strings.Contains(stack, "github.com/jackc/pgconn.(*PgConn).asyncClose.func1") ||
 			// Ignore pgconn which creates a goroutine to watch context cancellation.
 			strings.Contains(stack, "github.com/jackc/pgconn/internal/ctxwatch.(*ContextWatcher).Watch.func1") ||
+			// Ignore pq goroutine that watches for context cancellation.
+			strings.Contains(stack, "github.com/lib/pq.(*conn).watchCancel") ||
 			// Seems to be gccgo specific.
 			(runtime.Compiler == "gccgo" && strings.Contains(stack, "testing.T.Parallel")) ||
 			// Ignore intentionally long-running logging goroutines that live for the