database/sql: correctly report MaxIdleClosed stat

Previously the MaxIdleClosed counter was incremented when added
to the free connection list, rather then when it wasn't added
to the free connection list. Flip this logic to correct.

Fixes #27792

Change-Id: I405302c14fb985369dab48fbe845e5651afc4ccf
Reviewed-on: https://go-review.googlesource.com/c/138578
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index 7537f87d47cf5ca55a19c0d1ef39f3d226417a7b..7c8d46b1008dc8a8e2df3327968e2262e7c18601 100644 (file)
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -1322,11 +1322,13 @@ func (db *DB) putConnDBLocked(dc *driverConn, err error) bool {
                        err:  err,
                }
                return true
-       } else if err == nil && !db.closed && db.maxIdleConnsLocked() > len(db.freeConn) {
-               db.freeConn = append(db.freeConn, dc)
+       } else if err == nil && !db.closed {
+               if db.maxIdleConnsLocked() > len(db.freeConn) {
+                       db.freeConn = append(db.freeConn, dc)
+                       db.startCleanerLocked()
+                       return true
+               }
                db.maxIdleClosed++
-               db.startCleanerLocked()
-               return true
        }
        return false
 }

diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index f194744aefe727b0b0e51039de90c0fb29ac4345..e52091c3afd0c36d46ba4c9baaa32d840ddd033f 100644 (file)
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -3415,6 +3415,58 @@ func TestConnectionLeak(t *testing.T) {
        wg.Wait()
 }
 
+func TestStatsMaxIdleClosedZero(t *testing.T) {
+       db := newTestDB(t, "people")
+       defer closeDB(t, db)
+
+       db.SetMaxOpenConns(1)
+       db.SetMaxIdleConns(1)
+       db.SetConnMaxLifetime(0)
+
+       preMaxIdleClosed := db.Stats().MaxIdleClosed
+
+       for i := 0; i < 10; i++ {
+               rows, err := db.Query("SELECT|people|name|")
+               if err != nil {
+                       t.Fatal(err)
+               }
+               rows.Close()
+       }
+
+       st := db.Stats()
+       maxIdleClosed := st.MaxIdleClosed - preMaxIdleClosed
+       t.Logf("MaxIdleClosed: %d", maxIdleClosed)
+       if maxIdleClosed != 0 {
+               t.Fatal("expected 0 max idle closed conns, got: ", maxIdleClosed)
+       }
+}
+
+func TestStatsMaxIdleClosedTen(t *testing.T) {
+       db := newTestDB(t, "people")
+       defer closeDB(t, db)
+
+       db.SetMaxOpenConns(1)
+       db.SetMaxIdleConns(0)
+       db.SetConnMaxLifetime(0)
+
+       preMaxIdleClosed := db.Stats().MaxIdleClosed
+
+       for i := 0; i < 10; i++ {
+               rows, err := db.Query("SELECT|people|name|")
+               if err != nil {
+                       t.Fatal(err)
+               }
+               rows.Close()
+       }
+
+       st := db.Stats()
+       maxIdleClosed := st.MaxIdleClosed - preMaxIdleClosed
+       t.Logf("MaxIdleClosed: %d", maxIdleClosed)
+       if maxIdleClosed != 10 {
+               t.Fatal("expected 0 max idle closed conns, got: ", maxIdleClosed)
+       }
+}
+
 type nvcDriver struct {
        fakeDriver
        skipNamedValueCheck bool