Gerrit Bot has uploaded this change for review.
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.
Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Daniel Theophanes.
Gerrit Bot uploaded patch set #2 to this 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.
Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Daniel Theophanes.
Patch set 2:Hold +1
1 comment:
Patchset:
This is new exported API, and as such should go through the proposal process. See https://go.dev/s/proposal-process. Thanks.
To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Daniel Theophanes, Ian Lance Taylor.
1 comment:
Patchset:
This is new exported API, and as such should go through the proposal process. See https://go. […]
Hi Ian, thank you for your feedback, just submitted one. https://github.com/golang/go/issues/52886
To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Daniel Theophanes, Ian Lance Taylor.
Gerrit Bot uploaded patch set #3 to this 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.
Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Daniel Theophanes, Ian Lance Taylor.
Gerrit Bot uploaded patch set #4 to this 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.
Attention is currently required from: Brad Fitzpatrick, Bryan Mills, Ian Lance Taylor.
Patch set 4:Run-TryBot +1
Gopher Robot abandoned this change.
To view, visit change 404935. To unsubscribe, or for help writing mail filters, visit settings.