Skip to content

Commit

Permalink
ccl/sqlproxyccl: fix TestConnectionMigration test flake
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jaylim-crl committed Jun 27, 2022
1 parent 6ca1864 commit 0b696bb
Showing 1 changed file with 4 additions and 12 deletions.
16 changes: 4 additions & 12 deletions pkg/ccl/sqlproxyccl/proxy_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand Down

0 comments on commit 0b696bb

Please sign in to comment.