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: TestMaxOpenConns failing #7532

Closed
gpaul opened this issue Mar 13, 2014 · 7 comments
Closed

database/sql: TestMaxOpenConns failing #7532

gpaul opened this issue Mar 13, 2014 · 7 comments
Assignees
Milestone

Comments

@gpaul
Copy link
Contributor

@gpaul gpaul commented Mar 13, 2014

go version devel +d8c952ef0940 Thu Mar 13 18:26:01 2014 +1100 linux/amd64

Steps to reproduce:

for i in `seq 1 5000` ; do  export GOMAXPROCS=$[ 1 + $[ RANDOM % 128 ] ]; echo
GOMAXPROCS=$GOMAXPROCS; go test -v database/sql; done 2>&1 | tee sql.log

=== RUN TestMaxOpenConns-4
--- FAIL: TestMaxOpenConns-4 (0.25 seconds)
    sql_test.go:983: open calls = 11
    sql_test.go:984: close calls = 1
    sql_test.go:985: db connections opened = 11; want <= 10
    sql_test.go:150: *sql.driverConn (0xc2082245a0) waiting for -> *sql.driverConn (0xc2082245a0)
    sql_test.go:150: *sql.driverConn (0xc2081f40c0) waiting for -> *sql.driverConn (0xc2081f40c0)
    sql_test.go:150: *sql.driverConn (0xc20808e120) waiting for -> *sql.driverConn (0xc20808e120)
    sql_test.go:150: *sql.driverConn (0xc20808e180) waiting for -> *sql.driverConn (0xc20808e180)
    sql_test.go:150: *sql.driverConn (0xc20808e1e0) waiting for -> *sql.driverConn (0xc20808e1e0)
    sql_test.go:150: *sql.Stmt (0xc20815f300) waiting for -> *sql.Stmt (0xc20815f300)
    sql_test.go:150: *sql.driverConn (0xc2081f4180) waiting for -> *sql.driverConn (0xc2081f4180)
    sql_test.go:150: *sql.driverConn (0xc208224600) waiting for -> *sql.driverConn (0xc208224600)
    sql_test.go:150: *sql.driverConn (0xc2081f4240) waiting for -> *sql.driverConn (0xc2081f4240)
    sql_test.go:150: *sql.driverConn (0xc2081f42a0) waiting for -> *sql.driverConn (0xc2081f42a0)
    sql_test.go:150: *sql.driverConn (0xc20808e360) waiting for -> *sql.driverConn (0xc20808e360)
@gpaul
Copy link
Contributor Author

@gpaul gpaul commented Apr 14, 2014

Comment 1:

Bump. This still fails on tip and reproduces very quickly with the provided loop. 
go version devel +f8ac12d0acae Mon Apr 14 13:03:03 2014 -0700 linux/amd64
@davecheney
Copy link
Contributor

@davecheney davecheney commented Apr 16, 2014

Comment 2:

Confirmed. I think we should make an effort to resolve this for 1.3

Labels changed: added release-go1.3, repo-main.

Status changed to Accepted.

@gopherbot
Copy link

@gopherbot gopherbot commented May 5, 2014

Comment 3 by james.chalfant:

I believe the problem is in database/sql.go, in here:
func (db *DB) conn() (*driverConn, error)
There's a sequence that runs roughly as follows.
1.) lock mutex, check db.numOpen and db.maxOpen to see if a new connection should be
opened, unlock mutex
2.) do work to open a connection
3.) lock mutex,  update db.numOpen, unlock mutex
If two goroutines simultaneously get inside conn(), they may both decide to open a
connection before either updates numOpen, allowing the number of open connections to
exceed db.maxOpen. 
One solution could be to increment db.numOpen in step #1 and decrement it if the open
fails; I've prepared a change that does this and the test case passes. 
Another would be to use db.pendingOpens as maybeOpenNewConnections and openNewConnection
do. 
If you agree that this is the root cause and that the suggested fix is appropriate,
please let me know and I'll submit a change for review.
Thanks, 
-James
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented May 7, 2014

Comment 4:

I have a fix for this to be sent out shortly. Thanks for the repro & diagnosis!

Owner changed to @bradfitz.

Status changed to Started.

@gopherbot
Copy link

@gopherbot gopherbot commented May 7, 2014

Comment 5:

CL https://golang.org/cl/95130043 mentions this issue.
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented May 7, 2014

Comment 6:

This issue was closed by revision ce6b75d.

Status changed to Fixed.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented May 24, 2014

Comment 7:

Issue #7798 has been merged into this issue.

@gpaul gpaul added fixed labels May 24, 2014
@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.