From e10c84993d2b99d9347bddb31c0e3cc3e2393fd1 Mon Sep 17 00:00:00 2001 From: Jay Date: Mon, 27 Jun 2022 20:03:50 +0000 Subject: [PATCH] ccl/sqlproxyccl: fix TestConnectionMigration test flake Fixes #83096. It appears that database/sql does not provide any thread-safe guarantees for methods on the Conn type except the Close method. This commit fixes a test-only issue that causes a panic when there's a race between internal Conn methods by ensuring that Conn methods are used in a single-threaded way. Release note: None Release justification: sqlproxy only test change. --- pkg/ccl/sqlproxyccl/proxy_handler_test.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/pkg/ccl/sqlproxyccl/proxy_handler_test.go b/pkg/ccl/sqlproxyccl/proxy_handler_test.go index 6e9ac3ed6c32..efea8d5a4504 100644 --- a/pkg/ccl/sqlproxyccl/proxy_handler_test.go +++ b/pkg/ccl/sqlproxyccl/proxy_handler_test.go @@ -1230,10 +1230,8 @@ func TestConnectionMigration(t *testing.T) { conn, err := db.Conn(tCtx) require.NoError(t, err) - // Spin up a goroutine to trigger the initial connection. - go func() { - _ = conn.PingContext(tCtx) - }() + // Trigger the initial connection. + require.NoError(t, conn.PingContext(tCtx)) var f *forwarder require.Eventually(t, func() bool { @@ -1283,16 +1281,10 @@ func TestConnectionMigration(t *testing.T) { // one test. <-goCh time.Sleep(2 * time.Second) - // This should be an error because the transfer timed out. + // This should be an error because the transfer timed out. Connection + // should automatically be closed. require.Error(t, f.TransferConnection()) - // Connection should be closed because this is a non-recoverable error, - // i.e. timeout after sending the request, but before fully receiving - // its response. - err = conn.PingContext(tCtx) - require.Error(t, err) - require.Regexp(t, "(closed|bad connection)", err.Error()) - select { case <-time.After(10 * time.Second): t.Fatalf("require that pg_sleep query terminates")