From 0b696bbb4a16f91dd7823f7476b53c6e657bbc93 Mon Sep 17 00:00:00 2001 From: Jay Date: Mon, 27 Jun 2022 16:03:50 -0400 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 fdd5ec48b9ad..2c167b746014 100644 --- a/pkg/ccl/sqlproxyccl/proxy_handler_test.go +++ b/pkg/ccl/sqlproxyccl/proxy_handler_test.go @@ -1280,10 +1280,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 { @@ -1333,16 +1331,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")