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.

Sign up for GitHub

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

Jump to bottom

database/sql: prepared statements leak connections with high concurrency #5323

Closed
snaury opened this issue Apr 20, 2013 · 9 comments
Closed

database/sql: prepared statements leak connections with high concurrency #5323

snaury opened this issue Apr 20, 2013 · 9 comments
Labels
FrozenDueToAge

Comments

@snaury
Copy link
Contributor

snaury commented Apr 20, 2013 

When using database/sql with prepared statements and concurrency higher that
MaxIdleConns connections are leaked and recreated until database cannot accept anymore.
See https://groups.google.com/d/msg/golang-nuts/-YbB2Qjg41g/Afuaf8nSmGEJ for a related
discussion.

The problem is that all connections that have ever been used by Stmt are marked as
dependent on that Stmt, and that dependency is only removed when Stmt is closed. When
connection, that was over MaxIdleConns limit, is returned, it is not actually closed,
because it is still used by Stmt. Later, when Stmt tries to find a free connection, is
sees that connection is not free and doesn't use it. This causes database/sql to create
driver connections over and over again, until underlying database refuses and replies
with errors.

I think dependency should only be added in Stmt.connStmt and removed in releaseConn
callbacks, because prepared statements don't really appear to "use" the
connection, merely "cache" it. On the other hand such solution would mean that
drivers's Stmt.Close is never called.
@bradfitz
Copy link
Contributor

Comment 1:

Ouch, this is terrible, sorry.  I agree it makes any halfway-normal workload fall apart
quickly and hold open connections forever.
I can reproduce and have a test for it now.
I started working on a fix.
If I can't this into Go 1.1, I'll try to get it into the most immediate point release.

Labels changed: added packagebug, removed priority-triage.

Owner changed to @bradfitz.

Status changed to Accepted.

@bradfitz
Copy link
Contributor

Comment 2:

snaury, thanks again for the bug report.
Mailed out https://golang.org/cl/8836045/
Could you try it out and let me know if it fixes things for you?

@bradfitz
Copy link
Contributor

Comment 3:

This issue was closed by revision 277047f.

Status changed to Fixed.

@gopherbot
Copy link
Contributor

Comment 4 by baron.schwartz:

Did this get into Go 1.1?

@julienschmidt
Copy link
Contributor

Comment 5:

Yes. Can you still observe a leak with Go 1.1?

@gopherbot
Copy link
Contributor

Comment 6 by baron.schwartz:

I might have a problem elsewhere in the code. I will confirm if I can reproduce the
problem and it's still this issue.

@gopherbot
Copy link
Contributor

Comment 7 by dieterplaetinck:

Hey, I think I may be seeing this problem.
note: I'm a newbie, so maybe I'm doing something wrong!
note also: I'm using this through https://github.com/go-sql-driver/mysql
but if you look at this code:
https://github.com/vimeo/carbon-tagger/blob/master/carbon-tagger.go#L115
which is basically a function that creates some prepared statements and reads structs
from a channel, for every struct it reads, it executes a few of the prepared statements.
I was trying to find out if I should explicitly run close anywhere but I don't think so,
since I want to reuse the prepared statements. also note that they are all inserts,
except for one select which only returns one row so I run QueryRow on it.
what I notice is that when the code runs, it generates a large amount of new connections
to mysql, and when all items are processed, the amount of connections gradually drops
back to 0 (!)
I have db.SetMaxIdleConns(80) but that doesn't seem to do much, it goes way higher than
that. before I hit it, I reach "max fd's reached" and/or "mysql error too many
connections"

@gopherbot
Copy link
Contributor

Comment 8 by dieterplaetinck:

~ ❯❯❯ go version
go version go1.1 linux/amd64

@gopherbot
Copy link
Contributor

Comment 9 by dieterplaetinck:

ignore what i said. i only have 1 connection to mysql, i was looking at different tcp
sockets. sorry!

@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bradfitz @snaury @julienschmidt @gopherbot