database/sql: Retry with a fresh connection after maxBadConnRetries
Previously if the connection pool was larger than maxBadConnRetries
and there were a lot of bad connections in the pool (for example if
the database server was restarted), a query might have failed with an
ErrBadConn unnecessarily. Instead of trying to guess how many times
to retry, try maxBadConnRetries times and then force a fresh
connection to be used for the last attempt. At the same time, lower
maxBadConnRetries to a smaller value now that it's not that important
to retry so many times from the free connection list.
Fixes #8834
Change-Id: I6542f151a766a658980fb396fa4880ecf5874e3d
Reviewed-on: https://go-review.googlesource.com/2034
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index 45554c6..94f80a6 100644
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -1085,17 +1085,17 @@
db.SetMaxOpenConns(3)
- conn0, err := db.conn()
+ conn0, err := db.conn(cachedOrNewConn)
if err != nil {
t.Fatalf("db open conn fail: %v", err)
}
- conn1, err := db.conn()
+ conn1, err := db.conn(cachedOrNewConn)
if err != nil {
t.Fatalf("db open conn fail: %v", err)
}
- conn2, err := db.conn()
+ conn2, err := db.conn(cachedOrNewConn)
if err != nil {
t.Fatalf("db open conn fail: %v", err)
}
@@ -1385,6 +1385,79 @@
}
}
+// Test cases where there's more than maxBadConnRetries bad connections in the
+// pool (issue 8834)
+func TestManyErrBadConn(t *testing.T) {
+ manyErrBadConnSetup := func() *DB {
+ db := newTestDB(t, "people")
+
+ nconn := maxBadConnRetries + 1
+ db.SetMaxIdleConns(nconn)
+ db.SetMaxOpenConns(nconn)
+ // open enough connections
+ func() {
+ for i := 0; i < nconn; i++ {
+ rows, err := db.Query("SELECT|people|age,name|")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer rows.Close()
+ }
+ }()
+
+ if db.numOpen != nconn {
+ t.Fatalf("unexpected numOpen %d (was expecting %d)", db.numOpen, nconn)
+ } else if len(db.freeConn) != nconn {
+ t.Fatalf("unexpected len(db.freeConn) %d (was expecting %d)", len(db.freeConn), nconn)
+ }
+ for _, conn := range db.freeConn {
+ conn.ci.(*fakeConn).stickyBad = true
+ }
+ return db
+ }
+
+ // Query
+ db := manyErrBadConnSetup()
+ defer closeDB(t, db)
+ rows, err := db.Query("SELECT|people|age,name|")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = rows.Close(); err != nil {
+ t.Fatal(err)
+ }
+
+ // Exec
+ db = manyErrBadConnSetup()
+ defer closeDB(t, db)
+ _, err = db.Exec("INSERT|people|name=Julia,age=19")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ // Begin
+ db = manyErrBadConnSetup()
+ defer closeDB(t, db)
+ tx, err := db.Begin()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = tx.Rollback(); err != nil {
+ t.Fatal(err)
+ }
+
+ // Prepare
+ db = manyErrBadConnSetup()
+ defer closeDB(t, db)
+ stmt, err := db.Prepare("SELECT|people|age,name|")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err = stmt.Close(); err != nil {
+ t.Fatal(err)
+ }
+}
+
// golang.org/issue/5781
func TestErrBadConnReconnect(t *testing.T) {
db := newTestDB(t, "foo")
