[go] database/sql: add SetMaxBadConnRetries

80 views
Skip to first unread message

Gerrit Bot (Gerrit)

unread,
May 8, 2022, 2:40:00 AM5/8/22
to goph...@pubsubhelper.golang.org, Jinzhu Zhang, golang-co...@googlegroups.com

Gerrit Bot has uploaded this change for review.

View Change

database/sql: add SetMaxBadConnRetries

Add the ability to sets the number of maximum retries if the driver returns driver.ErrBadConn.

This can allow us to skip retry those long running SQL queries killed by db server.

Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
GitHub-Last-Rev: c340b1338c225523b0c810424016527bcad5462b
GitHub-Pull-Request: golang/go#52771
---
M src/database/sql/sql.go
M src/database/sql/sql_test.go
2 files changed, 113 insertions(+), 101 deletions(-)

diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index f408a34..a07753e 100644
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -487,6 +487,11 @@
maxIdleTimeClosed int64 // Total number of connections closed due to idle time.
maxLifetimeClosed int64 // Total number of connections closed due to max connection lifetime limit.

+ // maxBadConnRetries is the number of maximum retries if the driver returns
+ // driver.ErrBadConn to signal a broken connection before forcing a new
+ // connection to be opened.
+ maxBadConnRetries int
+
stop func() // stop cancels the connection opener.
}

@@ -847,17 +852,12 @@
func (db *DB) PingContext(ctx context.Context) error {
var dc *driverConn
var err error
- var isBadConn bool
- for i := 0; i < maxBadConnRetries; i++ {
- dc, err = db.conn(ctx, cachedOrNewConn)
- isBadConn = errors.Is(err, driver.ErrBadConn)
- if !isBadConn {
- break
- }
- }
- if isBadConn {
- dc, err = db.conn(ctx, alwaysNewConn)
- }
+
+ err = db.retry(func(strategy connReuseStrategy) error {
+ dc, err = db.conn(ctx, strategy)
+ return err
+ })
+
if err != nil {
return err
}
@@ -1048,6 +1048,49 @@
db.startCleanerLocked()
}

+// SetMaxBadConnRetries sets the number of maximum retries if the driver returns
+// driver.ErrBadConn to signal a broken connection before forcing a new
+// connection to be opened.
+//
+// If n <= 0, no retry when driver.ErrBadConn happen
+//
+// The default max bad retries connections is currently 2. This may change in
+// a future release.
+func (db *DB) SetMaxBadConnRetries(n int) {
+ db.mu.Lock()
+ db.maxBadConnRetries = n
+ db.mu.Unlock()
+}
+
+const defaultMaxBadConnRetries = 2
+
+func (db *DB) retry(fn func(strategy connReuseStrategy) error) error {
+ var err error
+ var isBadConn = true
+
+ maxBadConnRetries := db.maxBadConnRetries
+ if maxBadConnRetries == 0 {
+ maxBadConnRetries = defaultMaxBadConnRetries
+ }
+
+ for i := 0; i < maxBadConnRetries; i++ {
+ err = fn(cachedOrNewConn)
+ isBadConn = err != nil && errors.Is(err, driver.ErrBadConn)
+ if !isBadConn {
+ break
+ }
+ }
+
+ if isBadConn {
+ if maxBadConnRetries > 0 {
+ return fn(alwaysNewConn)
+ }
+ return fn(cachedOrNewConn)
+ }
+
+ return err
+}
+
// startCleanerLocked starts connectionCleaner if needed.
func (db *DB) startCleanerLocked() {
if (db.maxLifetime > 0 || db.maxIdleTime > 0) && db.numOpen > 0 && db.cleanerCh == nil {
@@ -1535,11 +1578,6 @@
return false
}

-// maxBadConnRetries is the number of maximum retries if the driver returns
-// driver.ErrBadConn to signal a broken connection before forcing a new
-// connection to be opened.
-const maxBadConnRetries = 2
-
// PrepareContext creates a prepared statement for later queries or executions.
// Multiple queries or executions may be run concurrently from the
// returned statement.
@@ -1551,17 +1589,11 @@
func (db *DB) PrepareContext(ctx context.Context, query string) (*Stmt, error) {
var stmt *Stmt
var err error
- var isBadConn bool
- for i := 0; i < maxBadConnRetries; i++ {
- stmt, err = db.prepare(ctx, query, cachedOrNewConn)
- isBadConn = errors.Is(err, driver.ErrBadConn)
- if !isBadConn {
- break
- }
- }
- if isBadConn {
- return db.prepare(ctx, query, alwaysNewConn)
- }
+ err = db.retry(func(strategy connReuseStrategy) error {
+ stmt, err = db.prepare(ctx, query, strategy)
+ return err
+ })
+
return stmt, err
}

@@ -1629,17 +1661,11 @@
func (db *DB) ExecContext(ctx context.Context, query string, args ...any) (Result, error) {
var res Result
var err error
- var isBadConn bool
- for i := 0; i < maxBadConnRetries; i++ {
- res, err = db.exec(ctx, query, args, cachedOrNewConn)
- isBadConn = errors.Is(err, driver.ErrBadConn)
- if !isBadConn {
- break
- }
- }
- if isBadConn {
- return db.exec(ctx, query, args, alwaysNewConn)
- }
+ err = db.retry(func(strategy connReuseStrategy) error {
+ res, err = db.exec(ctx, query, args, strategy)
+ return err
+ })
+
return res, err
}

@@ -1704,17 +1730,11 @@
func (db *DB) QueryContext(ctx context.Context, query string, args ...any) (*Rows, error) {
var rows *Rows
var err error
- var isBadConn bool
- for i := 0; i < maxBadConnRetries; i++ {
- rows, err = db.query(ctx, query, args, cachedOrNewConn)
- isBadConn = errors.Is(err, driver.ErrBadConn)
- if !isBadConn {
- break
- }
- }
- if isBadConn {
- return db.query(ctx, query, args, alwaysNewConn)
- }
+ err = db.retry(func(strategy connReuseStrategy) error {
+ rows, err = db.query(ctx, query, args, strategy)
+ return err
+ })
+
return rows, err
}

@@ -1841,17 +1861,11 @@
func (db *DB) BeginTx(ctx context.Context, opts *TxOptions) (*Tx, error) {
var tx *Tx
var err error
- var isBadConn bool
- for i := 0; i < maxBadConnRetries; i++ {
- tx, err = db.begin(ctx, opts, cachedOrNewConn)
- isBadConn = errors.Is(err, driver.ErrBadConn)
- if !isBadConn {
- break
- }
- }
- if isBadConn {
- return db.begin(ctx, opts, alwaysNewConn)
- }
+ err = db.retry(func(strategy connReuseStrategy) error {
+ tx, err = db.begin(ctx, opts, strategy)
+ return err
+ })
+
return tx, err
}

@@ -1922,17 +1936,11 @@
func (db *DB) Conn(ctx context.Context) (*Conn, error) {
var dc *driverConn
var err error
- var isBadConn bool
- for i := 0; i < maxBadConnRetries; i++ {
- dc, err = db.conn(ctx, cachedOrNewConn)
- isBadConn = errors.Is(err, driver.ErrBadConn)
- if !isBadConn {
- break
- }
- }
- if isBadConn {
- dc, err = db.conn(ctx, alwaysNewConn)
- }
+ err = db.retry(func(strategy connReuseStrategy) error {
+ dc, err = db.conn(ctx, strategy)
+ return err
+ })
+
if err != nil {
return nil, err
}
@@ -2621,26 +2629,18 @@
defer s.closemu.RUnlock()

var res Result
- strategy := cachedOrNewConn
- for i := 0; i < maxBadConnRetries+1; i++ {
- if i == maxBadConnRetries {
- strategy = alwaysNewConn
- }
+ err := s.db.retry(func(strategy connReuseStrategy) error {
dc, releaseConn, ds, err := s.connStmt(ctx, strategy)
if err != nil {
- if errors.Is(err, driver.ErrBadConn) {
- continue
- }
- return nil, err
+ return err
}

res, err = resultFromStatement(ctx, dc.ci, ds, args...)
releaseConn(err)
- if !errors.Is(err, driver.ErrBadConn) {
- return res, err
- }
- }
- return nil, driver.ErrBadConn
+ return err
+ })
+
+ return res, err
}

// Exec executes a prepared statement with the given arguments and
@@ -2769,24 +2769,19 @@
defer s.closemu.RUnlock()

var rowsi driver.Rows
- strategy := cachedOrNewConn
- for i := 0; i < maxBadConnRetries+1; i++ {
- if i == maxBadConnRetries {
- strategy = alwaysNewConn
- }
+ var rows *Rows
+
+ err := s.db.retry(func(strategy connReuseStrategy) error {
dc, releaseConn, ds, err := s.connStmt(ctx, strategy)
if err != nil {
- if errors.Is(err, driver.ErrBadConn) {
- continue
- }
- return nil, err
+ return err
}

rowsi, err = rowsiFromStatement(ctx, dc.ci, ds, args...)
if err == nil {
// Note: ownership of ci passes to the *Rows, to be freed
// with releaseConn.
- rows := &Rows{
+ rows = &Rows{
dc: dc,
rowsi: rowsi,
// releaseConn set below
@@ -2806,15 +2801,14 @@
txctx = s.cg.txCtx()
}
rows.initContextClose(ctx, txctx)
- return rows, nil
+ return nil
}

releaseConn(err)
- if !errors.Is(err, driver.ErrBadConn) {
- return nil, err
- }
- }
- return nil, driver.ErrBadConn
+ return err
+ })
+
+ return rows, err
}

// Query executes a prepared query statement with the given arguments
diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index a921dd5..ed6cf37 100644
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -2660,6 +2660,9 @@
f(db)
}

+ var maxBadConnRetries = 2
+ db.SetMaxBadConnRetries(maxBadConnRetries)
+
nconn := maxBadConnRetries + 1
db.SetMaxIdleConns(nconn)
db.SetMaxOpenConns(nconn)

To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
Gerrit-Change-Number: 404935
Gerrit-PatchSet: 1
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-CC: Jinzhu Zhang <wos...@gmail.com>
Gerrit-MessageType: newchange

Gerrit Bot (Gerrit)

unread,
May 10, 2022, 4:57:06 AM5/10/22
to Jinzhu Zhang, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Daniel Theophanes.

Gerrit Bot uploaded patch set #2 to this change.

View Change

database/sql: add SetMaxBadConnRetries

Add the ability to sets the number of maximum retries if the driver returns driver.ErrBadConn.

This can allow us to skip retry those long running SQL queries killed by db server.

Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
GitHub-Last-Rev: e4f95d94daa988ce2c7350bb8843c17f832114c4

GitHub-Pull-Request: golang/go#52771
---
M src/database/sql/sql.go
M src/database/sql/sql_test.go
2 files changed, 115 insertions(+), 106 deletions(-)

To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
Gerrit-Change-Number: 404935
Gerrit-PatchSet: 2
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Bryan Mills <bcm...@google.com>
Gerrit-Reviewer: Daniel Theophanes <kard...@gmail.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-CC: Jinzhu Zhang <wos...@gmail.com>
Gerrit-CC: Kevin Burke <ke...@burke.dev>
Gerrit-Attention: Bryan Mills <bcm...@google.com>
Gerrit-Attention: Daniel Theophanes <kard...@gmail.com>
Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
Gerrit-MessageType: newpatchset

Ian Lance Taylor (Gerrit)

unread,
May 10, 2022, 6:18:23 PM5/10/22
to Gerrit Bot, Jinzhu Zhang, goph...@pubsubhelper.golang.org, Ian Lance Taylor, Bryan Mills, Brad Fitzpatrick, Daniel Theophanes, Kevin Burke, Gopher Robot, golang-co...@googlegroups.com

Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Daniel Theophanes.

Patch set 2:Hold +1

View Change

1 comment:

To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
Gerrit-Change-Number: 404935
Gerrit-PatchSet: 2
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Bryan Mills <bcm...@google.com>
Gerrit-Reviewer: Daniel Theophanes <kard...@gmail.com>
Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-CC: Jinzhu Zhang <wos...@gmail.com>
Gerrit-CC: Kevin Burke <ke...@burke.dev>
Gerrit-Attention: Bryan Mills <bcm...@google.com>
Gerrit-Attention: Daniel Theophanes <kard...@gmail.com>
Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Comment-Date: Tue, 10 May 2022 22:18:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Jinzhu Zhang (Gerrit)

unread,
May 13, 2022, 6:00:18 AM5/13/22
to Gerrit Bot, goph...@pubsubhelper.golang.org, Ian Lance Taylor, Bryan Mills, Brad Fitzpatrick, Daniel Theophanes, Kevin Burke, Gopher Robot, golang-co...@googlegroups.com

Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Daniel Theophanes, Ian Lance Taylor.

View Change

1 comment:

To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
Gerrit-Change-Number: 404935
Gerrit-PatchSet: 2
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Bryan Mills <bcm...@google.com>
Gerrit-Reviewer: Daniel Theophanes <kard...@gmail.com>
Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-CC: Jinzhu Zhang <wos...@gmail.com>
Gerrit-CC: Kevin Burke <ke...@burke.dev>
Gerrit-Attention: Bryan Mills <bcm...@google.com>
Gerrit-Attention: Daniel Theophanes <kard...@gmail.com>
Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
Gerrit-Comment-Date: Fri, 13 May 2022 10:00:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Ian Lance Taylor <ia...@golang.org>
Gerrit-MessageType: comment

Gerrit Bot (Gerrit)

unread,
May 13, 2022, 6:29:06 AM5/13/22
to Jinzhu Zhang, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Daniel Theophanes, Ian Lance Taylor.

Gerrit Bot uploaded patch set #3 to this change.

View Change

database/sql: add SetMaxBadConnRetries

Add the ability to sets the number of maximum retries if the driver returns driver.ErrBadConn.

This can allow us to skip retry those long running SQL queries killed by db server.

Fixes #52886


Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
GitHub-Last-Rev: e4f95d94daa988ce2c7350bb8843c17f832114c4
GitHub-Pull-Request: golang/go#52771
---
M src/database/sql/sql.go
M src/database/sql/sql_test.go
2 files changed, 117 insertions(+), 106 deletions(-)

To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
Gerrit-Change-Number: 404935
Gerrit-PatchSet: 3
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Reviewer: Bryan Mills <bcm...@google.com>
Gerrit-Reviewer: Daniel Theophanes <kard...@gmail.com>
Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-CC: Jinzhu Zhang <wos...@gmail.com>
Gerrit-CC: Kevin Burke <ke...@burke.dev>
Gerrit-Attention: Bryan Mills <bcm...@google.com>
Gerrit-Attention: Daniel Theophanes <kard...@gmail.com>
Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
Gerrit-MessageType: newpatchset

Gerrit Bot (Gerrit)

unread,
May 19, 2022, 12:14:56 AM5/19/22
to Jinzhu Zhang, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Daniel Theophanes, Ian Lance Taylor.

Gerrit Bot uploaded patch set #4 to this change.

View Change

database/sql: add SetMaxBadConnRetries

Add the ability to sets the number of maximum retries if the driver returns driver.ErrBadConn.

This can allow us to skip retry those long running SQL queries killed by db server.

Fixes #52886

Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
GitHub-Last-Rev: bf8779b86eb41e336ecab39412e1670e965ca10e

GitHub-Pull-Request: golang/go#52771
---
M src/database/sql/sql.go
M src/database/sql/sql_test.go
2 files changed, 117 insertions(+), 106 deletions(-)

To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: master
Gerrit-Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
Gerrit-Change-Number: 404935
Gerrit-PatchSet: 4

Daniel Theophanes (Gerrit)

unread,
Jul 20, 2022, 3:07:18 PM7/20/22
to Gerrit Bot, Jinzhu Zhang, goph...@pubsubhelper.golang.org, Ian Lance Taylor, Bryan Mills, Brad Fitzpatrick, Kevin Burke, Gopher Robot, golang-co...@googlegroups.com

Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Ian Lance Taylor.

Patch set 4:Run-TryBot +1

View Change

    To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
    Gerrit-Change-Number: 404935
    Gerrit-PatchSet: 4
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Bryan Mills <bcm...@google.com>
    Gerrit-Reviewer: Daniel Theophanes <kard...@gmail.com>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-CC: Jinzhu Zhang <wos...@gmail.com>
    Gerrit-CC: Kevin Burke <ke...@burke.dev>
    Gerrit-Attention: Bryan Mills <bcm...@google.com>
    Gerrit-Attention: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>
    Gerrit-Comment-Date: Wed, 20 Jul 2022 19:07:13 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Gopher Robot (Gerrit)

    unread,
    Jul 25, 2022, 8:03:38 AM7/25/22
    to Gerrit Bot, Jinzhu Zhang, goph...@pubsubhelper.golang.org, Daniel Theophanes, Ian Lance Taylor, Bryan Mills, Brad Fitzpatrick, Kevin Burke, golang-co...@googlegroups.com

    Gopher Robot abandoned this change.

    View Change

    Abandoned GitHub PR golang/go#52771 has been closed.

    To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: go
    Gerrit-Branch: master
    Gerrit-Change-Id: I11916843b378dab28295906cb66d42a9d4d97608
    Gerrit-Change-Number: 404935
    Gerrit-PatchSet: 4
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Brad Fitzpatrick <brad...@golang.org>
    Gerrit-Reviewer: Bryan Mills <bcm...@google.com>
    Gerrit-Reviewer: Daniel Theophanes <kard...@gmail.com>
    Gerrit-Reviewer: Gopher Robot <go...@golang.org>
    Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>
    Gerrit-CC: Jinzhu Zhang <wos...@gmail.com>
    Gerrit-CC: Kevin Burke <ke...@burke.dev>
    Gerrit-MessageType: abandon
    Reply all
    Reply to author
    Forward
    0 new messages