Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

database/sql: Why do DB.QueryContext and Stmt.QueryContext have different retry logic? #20433

Closed
krzysztofdrys opened this issue May 19, 2017 · 1 comment
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@krzysztofdrys
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.8.1 darwin/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/krzysztofdrys/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/4j/1_3ww7d10vs1j9njy4sjrmzr0000gn/T/go-build315987963=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

Summary

We are getting problems with databse/sql/driver.ErrBadConn, but only if we use the prepared statements. It seems to me that this is because *DB.QueryContext retries with a fresh connection and *Stmt.QueryContext does not. I want to verify whether this is a bug or an expected behaviour,

When *DB.QueryContext receives driver.ErrBaddConn from the driver, it will retry twice (maxBadConnRetries) using cached connection (cachedOrNewConn strategy). If this fails, it will retry once more using new connection (alwaysNewConn strategy). The code is here: https://golang.org/src/database/sql/sql.go?s=7997:9207#L1223

*Stmt.QueryContext will also retry twice (again maxBadConnRetries) using the cached connection. If this fails, it will return driver.ErrBadConn, without retrying using the new connection. The code is here: https://golang.org/src/database/sql/sql.go?s=7997:9207#L1935 s.connStmt uses cachedOrNewConn strategy.

I think this is a bug (but I will be happy to hear otherwise), introduced in this commit: c468f94

Before this commit, maxBadConnRetries was 10 and retries where alway done using old (cached) connections. Commit changed the logic, decreasing the number of retires and adding one more retry on fresh connection. While decrease was global (both *DB and *Stmt where affected), retry-using-fresh-connection logic was applied only to *DB.

I first posted this on golang-nuts and then followed the instructions to post it here.

I have been looking at the code in datbase/sql quite a lot lately, so if this is a bug, I think I can prepare the patch.

@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 19, 2017
@bradfitz bradfitz added this to the Go1.9Maybe milestone May 19, 2017
@bradfitz bradfitz changed the title Why do DB.QueryContext and Stmt.QueryContext have different retry logic? database/sql: Why do DB.QueryContext and Stmt.QueryContext have different retry logic? May 19, 2017
@gopherbot
Copy link

CL https://golang.org/cl/43790 mentions this issue.

@golang golang locked and limited conversation to collaborators May 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants