database/sql: fix auto-reconnect in prepared statements This also fixes several connection leaks. Fixes #5718 R=bradfitz, adg CC=alberto.garcia.hierro, golang-dev https://golang.org/cl/14920046
diff --git a/src/pkg/database/sql/sql.go b/src/pkg/database/sql/sql.go index fae109f..df989cd 100644 --- a/src/pkg/database/sql/sql.go +++ b/src/pkg/database/sql/sql.go
@@ -256,7 +256,7 @@ // stmt closes if the conn is about to close anyway? For now // do the safe thing, in case stmts need to be closed. // - // TODO(bradfitz): after Go 1.1, closing driver.Stmts + // TODO(bradfitz): after Go 1.2, closing driver.Stmts // should be moved to driverStmt, using unique // *driverStmts everywhere (including from // *Stmt.connStmt, instead of returning a @@ -798,13 +798,17 @@ return false } +// maxBadConnRetries is the number of maximum retries if the driver returns +// driver.ErrBadConn to signal a broken connection. +const maxBadConnRetries = 10 + // Prepare creates a prepared statement for later queries or executions. // Multiple queries or executions may be run concurrently from the // returned statement. func (db *DB) Prepare(query string) (*Stmt, error) { var stmt *Stmt var err error - for i := 0; i < 10; i++ { + for i := 0; i < maxBadConnRetries; i++ { stmt, err = db.prepare(query) if err != driver.ErrBadConn { break @@ -846,7 +850,7 @@ func (db *DB) Exec(query string, args ...interface{}) (Result, error) { var res Result var err error - for i := 0; i < 10; i++ { + for i := 0; i < maxBadConnRetries; i++ { res, err = db.exec(query, args) if err != driver.ErrBadConn { break @@ -895,7 +899,7 @@ func (db *DB) Query(query string, args ...interface{}) (*Rows, error) { var rows *Rows var err error - for i := 0; i < 10; i++ { + for i := 0; i < maxBadConnRetries; i++ { rows, err = db.query(query, args) if err != driver.ErrBadConn { break @@ -983,7 +987,7 @@ func (db *DB) Begin() (*Tx, error) { var tx *Tx var err error - for i := 0; i < 10; i++ { + for i := 0; i < maxBadConnRetries; i++ { tx, err = db.begin() if err != driver.ErrBadConn { break @@ -1245,13 +1249,24 @@ func (s *Stmt) Exec(args ...interface{}) (Result, error) { s.closemu.RLock() defer s.closemu.RUnlock() - dc, releaseConn, si, err := s.connStmt() - if err != nil { - return nil, err - } - defer releaseConn(nil) - return resultFromStatement(driverStmt{dc, si}, args...) + var res Result + for i := 0; i < maxBadConnRetries; i++ { + dc, releaseConn, si, err := s.connStmt() + if err != nil { + if err == driver.ErrBadConn { + continue + } + return nil, err + } + + res, err = resultFromStatement(driverStmt{dc, si}, args...) + releaseConn(err) + if err != driver.ErrBadConn { + return res, err + } + } + return nil, driver.ErrBadConn } func resultFromStatement(ds driverStmt, args ...interface{}) (Result, error) { @@ -1329,26 +1344,21 @@ // Make a new conn if all are busy. // TODO(bradfitz): or wait for one? make configurable later? if !match { - for i := 0; ; i++ { - dc, err := s.db.conn() - if err != nil { - return nil, nil, nil, err - } - dc.Lock() - si, err := dc.prepareLocked(s.query) - dc.Unlock() - if err == driver.ErrBadConn && i < 10 { - continue - } - if err != nil { - return nil, nil, nil, err - } - s.mu.Lock() - cs = connStmt{dc, si} - s.css = append(s.css, cs) - s.mu.Unlock() - break + dc, err := s.db.conn() + if err != nil { + return nil, nil, nil, err } + dc.Lock() + si, err := dc.prepareLocked(s.query) + dc.Unlock() + if err != nil { + s.db.putConn(dc, err) + return nil, nil, nil, err + } + s.mu.Lock() + cs = connStmt{dc, si} + s.css = append(s.css, cs) + s.mu.Unlock() } conn := cs.dc @@ -1361,31 +1371,39 @@ s.closemu.RLock() defer s.closemu.RUnlock() - dc, releaseConn, si, err := s.connStmt() - if err != nil { - return nil, err - } + var rowsi driver.Rows + for i := 0; i < maxBadConnRetries; i++ { + dc, releaseConn, si, err := s.connStmt() + if err != nil { + if err == driver.ErrBadConn { + continue + } + return nil, err + } - ds := driverStmt{dc, si} - rowsi, err := rowsiFromStatement(ds, args...) - if err != nil { - releaseConn(err) - return nil, err - } + rowsi, err = rowsiFromStatement(driverStmt{dc, si}, args...) + if err == nil { + // Note: ownership of ci passes to the *Rows, to be freed + // with releaseConn. + rows := &Rows{ + dc: dc, + rowsi: rowsi, + // releaseConn set below + } + s.db.addDep(s, rows) + rows.releaseConn = func(err error) { + releaseConn(err) + s.db.removeDep(s, rows) + } + return rows, nil + } - // Note: ownership of ci passes to the *Rows, to be freed - // with releaseConn. - rows := &Rows{ - dc: dc, - rowsi: rowsi, - // releaseConn set below - } - s.db.addDep(s, rows) - rows.releaseConn = func(err error) { releaseConn(err) - s.db.removeDep(s, rows) + if err != driver.ErrBadConn { + return nil, err + } } - return rows, nil + return nil, driver.ErrBadConn } func rowsiFromStatement(ds driverStmt, args ...interface{}) (driver.Rows, error) {