From 3b9d947b2f4eb3bf7ff01eb33cd1e91bbce73c77 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 24 Oct 2017 18:43:07 +0000 Subject: Revert "database/sql: add driver.ResetSessioner and add pool support" This reverts commit 2620ac3aeafe75a62fa81bd5094a8e1e4ef1ca8b. Reason for revert: broke all the builds. Change-Id: I26fc09a13f5f80fa708de66c843442ff9d934694 Reviewed-on: https://go-review.googlesource.com/73050 Reviewed-by: Russ Cox --- src/database/sql/sql.go | 105 +++++------------------------------------------- 1 file changed, 10 insertions(+), 95 deletions(-) (limited to 'src/database/sql/sql.go') diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go index c17b2b543b..7c35710688 100644 --- a/src/database/sql/sql.go +++ b/src/database/sql/sql.go @@ -334,7 +334,6 @@ type DB struct { // It is closed during db.Close(). The close tells the connectionOpener // goroutine to exit. openerCh chan struct{} - resetterCh chan *driverConn closed bool dep map[finalCloser]depSet lastPut map[*driverConn]string // stacktrace of last conn's put; debug only @@ -342,8 +341,6 @@ type DB struct { maxOpen int // <= 0 means unlimited maxLifetime time.Duration // maximum amount of time a connection may be reused cleanerCh chan struct{} - - stop func() // stop cancels the connection opener and the session resetter. } // connReuseStrategy determines how (*DB).conn returns database connections. @@ -371,7 +368,6 @@ type driverConn struct { closed bool finalClosed bool // ci.Close has been called openStmt map[*driverStmt]bool - lastErr error // lastError captures the result of the session resetter. // guarded by db.mu inUse bool @@ -380,7 +376,7 @@ type driverConn struct { } func (dc *driverConn) releaseConn(err error) { - dc.db.putConn(dc, err, true) + dc.db.putConn(dc, err) } func (dc *driverConn) removeOpenStmt(ds *driverStmt) { @@ -421,19 +417,6 @@ func (dc *driverConn) prepareLocked(ctx context.Context, cg stmtConnGrabber, que return ds, nil } -// resetSession resets the connection session and sets the lastErr -// that is checked before returning the connection to another query. -// -// resetSession assumes that the embedded mutex is locked when the connection -// was returned to the pool. This unlocks the mutex. -func (dc *driverConn) resetSession(ctx context.Context) { - defer dc.Unlock() // In case of panic. - if dc.closed { // Check if the database has been closed. - return - } - dc.lastErr = dc.ci.(driver.ResetSessioner).ResetSession(ctx) -} - // the dc.db's Mutex is held. func (dc *driverConn) closeDBLocked() func() error { dc.Lock() @@ -621,18 +604,14 @@ func (t dsnConnector) Driver() driver.Driver { // function should be called just once. It is rarely necessary to // close a DB. func OpenDB(c driver.Connector) *DB { - ctx, cancel := context.WithCancel(context.Background()) db := &DB{ connector: c, openerCh: make(chan struct{}, connectionRequestQueueSize), - resetterCh: make(chan *driverConn, 50), lastPut: make(map[*driverConn]string), connRequests: make(map[uint64]chan connRequest), - stop: cancel, } - go db.connectionOpener(ctx) - go db.connectionResetter(ctx) + go db.connectionOpener() return db } @@ -714,6 +693,7 @@ func (db *DB) Close() error { db.mu.Unlock() return nil } + close(db.openerCh) if db.cleanerCh != nil { close(db.cleanerCh) } @@ -734,7 +714,6 @@ func (db *DB) Close() error { err = err1 } } - db.stop() return err } @@ -922,39 +901,18 @@ func (db *DB) maybeOpenNewConnections() { } // Runs in a separate goroutine, opens new connections when requested. -func (db *DB) connectionOpener(ctx context.Context) { - for { - select { - case <-ctx.Done(): - return - case <-db.openerCh: - db.openNewConnection(ctx) - } - } -} - -// connectionResetter runs in a separate goroutine to reset connections async -// to exported API. -func (db *DB) connectionResetter(ctx context.Context) { - for { - select { - case <-ctx.Done(): - for dc := range db.resetterCh { - dc.Unlock() - } - return - case dc := <-db.resetterCh: - dc.resetSession(ctx) - } +func (db *DB) connectionOpener() { + for range db.openerCh { + db.openNewConnection() } } // Open one new connection -func (db *DB) openNewConnection(ctx context.Context) { +func (db *DB) openNewConnection() { // maybeOpenNewConnctions has already executed db.numOpen++ before it sent // on db.openerCh. This function must execute db.numOpen-- if the // connection fails or is closed before returning. - ci, err := db.connector.Connect(ctx) + ci, err := db.connector.Connect(context.Background()) db.mu.Lock() defer db.mu.Unlock() if db.closed { @@ -1029,14 +987,6 @@ func (db *DB) conn(ctx context.Context, strategy connReuseStrategy) (*driverConn conn.Close() return nil, driver.ErrBadConn } - // Lock around reading lastErr to ensure the session resetter finished. - conn.Lock() - err := conn.lastErr - conn.Unlock() - if err == driver.ErrBadConn { - conn.Close() - return nil, driver.ErrBadConn - } return conn, nil } @@ -1062,7 +1012,7 @@ func (db *DB) conn(ctx context.Context, strategy connReuseStrategy) (*driverConn default: case ret, ok := <-req: if ok { - db.putConn(ret.conn, ret.err, false) + db.putConn(ret.conn, ret.err) } } return nil, ctx.Err() @@ -1074,17 +1024,6 @@ func (db *DB) conn(ctx context.Context, strategy connReuseStrategy) (*driverConn ret.conn.Close() return nil, driver.ErrBadConn } - if ret.conn == nil { - return nil, ret.err - } - // Lock around reading lastErr to ensure the session resetter finished. - ret.conn.Lock() - err := ret.conn.lastErr - ret.conn.Unlock() - if err == driver.ErrBadConn { - ret.conn.Close() - return nil, driver.ErrBadConn - } return ret.conn, ret.err } } @@ -1140,7 +1079,7 @@ const debugGetPut = false // putConn adds a connection to the db's free pool. // err is optionally the last error that occurred on this connection. -func (db *DB) putConn(dc *driverConn, err error, resetSession bool) { +func (db *DB) putConn(dc *driverConn, err error) { db.mu.Lock() if !dc.inUse { if debugGetPut { @@ -1171,35 +1110,11 @@ func (db *DB) putConn(dc *driverConn, err error, resetSession bool) { if putConnHook != nil { putConnHook(db, dc) } - if resetSession { - if _, resetSession = dc.ci.(driver.ResetSessioner); resetSession { - // Lock the driverConn here so it isn't released until - // the connection is reset. - // The lock must be taken before the connection is put into - // the pool to prevent it from being taken out before it is reset. - dc.Lock() - } - } added := db.putConnDBLocked(dc, nil) db.mu.Unlock() if !added { - if resetSession { - dc.Unlock() - } dc.Close() - return - } - if !resetSession { - return - } - select { - default: - // If the resetterCh is blocking then mark the connection - // as bad and continue on. - dc.lastErr = driver.ErrBadConn - dc.Unlock() - case db.resetterCh <- dc: } } -- cgit v1.3