From 6e5fa60c4cc3977489a33f364ee44cac85049abc Mon Sep 17 00:00:00 2001 From: Jack Christensen Date: Sat, 12 Sep 2015 19:32:55 -0500 Subject: [PATCH] Fix cases where net conn write failure was not marking connection as dead Also added loop to run these timing sensitive tests multiple times. --- conn.go | 2 + conn_pool_test.go | 113 ++++++++++++++++++++++++---------------------- conn_test.go | 39 +++++++++------- 3 files changed, 83 insertions(+), 71 deletions(-) diff --git a/conn.go b/conn.go index 8936e438..09c14578 100644 --- a/conn.go +++ b/conn.go @@ -483,6 +483,7 @@ func (c *Conn) Prepare(name, sql string) (ps *PreparedStatement, err error) { _, err = c.conn.Write(wbuf.buf) if err != nil { + c.die(err) return nil, err } @@ -544,6 +545,7 @@ func (c *Conn) Deallocate(name string) (err error) { _, err = c.conn.Write(wbuf.buf) if err != nil { + c.die(err) return err } diff --git a/conn_pool_test.go b/conn_pool_test.go index b6b7d98a..74c03a2b 100644 --- a/conn_pool_test.go +++ b/conn_pool_test.go @@ -229,64 +229,69 @@ func TestPoolAcquireAndReleaseCycleAutoConnect(t *testing.T) { func TestPoolReleaseDiscardsDeadConnections(t *testing.T) { t.Parallel() - maxConnections := 3 - pool := createConnPool(t, maxConnections) - defer pool.Close() + // Run timing sensitive test many times + for i := 0; i < 50; i++ { + func() { + maxConnections := 3 + pool := createConnPool(t, maxConnections) + defer pool.Close() - var c1, c2 *pgx.Conn - var err error - var stat pgx.ConnPoolStat + var c1, c2 *pgx.Conn + var err error + var stat pgx.ConnPoolStat + + if c1, err = pool.Acquire(); err != nil { + t.Fatalf("Unexpected error acquiring connection: %v", err) + } + defer func() { + if c1 != nil { + pool.Release(c1) + } + }() + + if c2, err = pool.Acquire(); err != nil { + t.Fatalf("Unexpected error acquiring connection: %v", err) + } + defer func() { + if c2 != nil { + pool.Release(c2) + } + }() + + if _, err = c2.Exec("select pg_terminate_backend($1)", c1.Pid); err != nil { + t.Fatalf("Unable to kill backend PostgreSQL process: %v", err) + } + + // do something with the connection so it knows it's dead + rows, _ := c1.Query("select 1") + rows.Close() + if rows.Err() == nil { + t.Fatal("Expected error but none occurred") + } + + if c1.IsAlive() { + t.Fatal("Expected connection to be dead but it wasn't") + } + + stat = pool.Stat() + if stat.CurrentConnections != 2 { + t.Fatalf("Unexpected CurrentConnections: %v", stat.CurrentConnections) + } + if stat.AvailableConnections != 0 { + t.Fatalf("Unexpected AvailableConnections: %v", stat.CurrentConnections) + } - if c1, err = pool.Acquire(); err != nil { - t.Fatalf("Unexpected error acquiring connection: %v", err) - } - defer func() { - if c1 != nil { pool.Release(c1) - } - }() + c1 = nil // so it doesn't get released again by the defer - if c2, err = pool.Acquire(); err != nil { - t.Fatalf("Unexpected error acquiring connection: %v", err) - } - defer func() { - if c2 != nil { - pool.Release(c2) - } - }() - - if _, err = c2.Exec("select pg_terminate_backend($1)", c1.Pid); err != nil { - t.Fatalf("Unable to kill backend PostgreSQL process: %v", err) - } - - // do something with the connection so it knows it's dead - rows, _ := c1.Query("select 1") - rows.Close() - if rows.Err() == nil { - t.Fatal("Expected error but none occurred") - } - - if c1.IsAlive() { - t.Fatal("Expected connection to be dead but it wasn't") - } - - stat = pool.Stat() - if stat.CurrentConnections != 2 { - t.Fatalf("Unexpected CurrentConnections: %v", stat.CurrentConnections) - } - if stat.AvailableConnections != 0 { - t.Fatalf("Unexpected AvailableConnections: %v", stat.CurrentConnections) - } - - pool.Release(c1) - c1 = nil // so it doesn't get released again by the defer - - stat = pool.Stat() - if stat.CurrentConnections != 1 { - t.Fatalf("Unexpected CurrentConnections: %v", stat.CurrentConnections) - } - if stat.AvailableConnections != 0 { - t.Fatalf("Unexpected AvailableConnections: %v", stat.CurrentConnections) + stat = pool.Stat() + if stat.CurrentConnections != 1 { + t.Fatalf("Unexpected CurrentConnections: %v", stat.CurrentConnections) + } + if stat.AvailableConnections != 0 { + t.Fatalf("Unexpected AvailableConnections: %v", stat.CurrentConnections) + } + }() } } diff --git a/conn_test.go b/conn_test.go index 3cc9aa5b..2e3f612e 100644 --- a/conn_test.go +++ b/conn_test.go @@ -945,27 +945,32 @@ func TestFatalRxError(t *testing.T) { func TestFatalTxError(t *testing.T) { t.Parallel() - conn := mustConnect(t, *defaultConnConfig) - defer closeConn(t, conn) + // Run timing sensitive test many times + for i := 0; i < 50; i++ { + func() { + conn := mustConnect(t, *defaultConnConfig) + defer closeConn(t, conn) - otherConn, err := pgx.Connect(*defaultConnConfig) - if err != nil { - t.Fatalf("Unable to establish connection: %v", err) - } - defer otherConn.Close() + otherConn, err := pgx.Connect(*defaultConnConfig) + if err != nil { + t.Fatalf("Unable to establish connection: %v", err) + } + defer otherConn.Close() - _, err = otherConn.Exec("select pg_terminate_backend($1)", conn.Pid) - if err != nil { - t.Fatalf("Unable to kill backend PostgreSQL process: %v", err) - } + _, err = otherConn.Exec("select pg_terminate_backend($1)", conn.Pid) + if err != nil { + t.Fatalf("Unable to kill backend PostgreSQL process: %v", err) + } - _, err = conn.Query("select 1") - if err == nil { - t.Fatal("Expected error but none occurred") - } + _, err = conn.Query("select 1") + if err == nil { + t.Fatal("Expected error but none occurred") + } - if conn.IsAlive() { - t.Fatal("Connection should not be live but was") + if conn.IsAlive() { + t.Fatalf("Connection should not be live but was. Previous Query err: %v", err) + } + }() } }