From 93aa2b2e809b33d6689b199019feeaab37258e8d Mon Sep 17 00:00:00 2001
From: Jack Christensen <jack@jackchristensen.com>
Date: Sat, 12 Sep 2015 19:39:37 -0500
Subject: [PATCH] ConnPool.Begin retry logic checks connection IsAlive

ErrDeadConn is returned when calling an already dead connection. But the initial failure returns the real error. So we check for IsAlive instead of ErrDeadConn.

Added test for ConnPool.Begin retry logic.
---
 conn_pool.go      | 16 ++++++++----
 conn_pool_test.go | 65 +++++++++++++++++++++++++----------------------
 2 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/conn_pool.go b/conn_pool.go
index 0e04268e..cd8b7f63 100644
--- a/conn_pool.go
+++ b/conn_pool.go
@@ -228,13 +228,19 @@ func (p *ConnPool) BeginIso(iso string) (*Tx, error) {
 		}
 
 		tx, err := c.BeginIso(iso)
-		if err == ErrDeadConn {
-			p.Release(c)
-			continue
-		}
 		if err != nil {
+			alive := c.IsAlive()
 			p.Release(c)
-			return nil, err
+
+			// If connection is still alive then the error is not something trying
+			// again on a new connection would fix, so just return the error. But
+			// if the connection is dead try to acquire a new connection and try
+			// again.
+			if alive {
+				return nil, err
+			} else {
+				continue
+			}
 		}
 
 		tx.pool = p
diff --git a/conn_pool_test.go b/conn_pool_test.go
index 74c03a2b..73519855 100644
--- a/conn_pool_test.go
+++ b/conn_pool_test.go
@@ -363,41 +363,46 @@ func TestConnPoolTransactionIso(t *testing.T) {
 func TestConnPoolBeginRetry(t *testing.T) {
 	t.Parallel()
 
-	pool := createConnPool(t, 2)
-	defer pool.Close()
+	// Run timing sensitive test many times
+	for i := 0; i < 50; i++ {
+		func() {
+			pool := createConnPool(t, 2)
+			defer pool.Close()
 
-	killerConn, err := pool.Acquire()
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer pool.Release(killerConn)
+			killerConn, err := pool.Acquire()
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer pool.Release(killerConn)
 
-	victimConn, err := pool.Acquire()
-	if err != nil {
-		t.Fatal(err)
-	}
-	pool.Release(victimConn)
+			victimConn, err := pool.Acquire()
+			if err != nil {
+				t.Fatal(err)
+			}
+			pool.Release(victimConn)
 
-	// Terminate connection that was released to pool
-	if _, err = killerConn.Exec("select pg_terminate_backend($1)", victimConn.Pid); err != nil {
-		t.Fatalf("Unable to kill backend PostgreSQL process: %v", err)
-	}
+			// Terminate connection that was released to pool
+			if _, err = killerConn.Exec("select pg_terminate_backend($1)", victimConn.Pid); err != nil {
+				t.Fatalf("Unable to kill backend PostgreSQL process: %v", err)
+			}
 
-	// Since victimConn is the only available connection in the pool, pool.Begin should
-	// try to use it, fail, and allocate another connection
-	tx, err := pool.Begin()
-	if err != nil {
-		t.Fatalf("pool.Begin failed: %v", err)
-	}
-	defer tx.Rollback()
+			// Since victimConn is the only available connection in the pool, pool.Begin should
+			// try to use it, fail, and allocate another connection
+			tx, err := pool.Begin()
+			if err != nil {
+				t.Fatalf("pool.Begin failed: %v", err)
+			}
+			defer tx.Rollback()
 
-	var txPid int32
-	err = tx.QueryRow("select pg_backend_pid()").Scan(&txPid)
-	if err != nil {
-		t.Fatalf("tx.QueryRow Scan failed: %v", err)
-	}
-	if txPid == victimConn.Pid {
-		t.Error("Expected txPid to defer from killed conn pid, but it didn't")
+			var txPid int32
+			err = tx.QueryRow("select pg_backend_pid()").Scan(&txPid)
+			if err != nil {
+				t.Fatalf("tx.QueryRow Scan failed: %v", err)
+			}
+			if txPid == victimConn.Pid {
+				t.Error("Expected txPid to defer from killed conn pid, but it didn't")
+			}
+		}()
 	}
 }