| Age | Commit message (Collapse) | Author |
|
This is the main point of confusion and the emphasis of
a recent Gophercon talk.
Fixes #5886. (mostly fixed in previous commits)
LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/100560043
|
|
Existing test TestMaxOpenConns was failing occasionally, especially
with higher values of GOMAXPROCS.
Fixes #7532
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/95130043
|
|
LGTM=ruiu, bradfitz
R=golang-codereviews, bradfitz, ruiu
CC=golang-codereviews
https://golang.org/cl/91840044
|
|
Strictly speaking, it's not necessary in example_test.go, as the
Rows.Close docs say that "If Next returns false, the Rows are closed
automatically". However, if the for loop breaks or returns early, it's
not obvious that you'll leak unless you explicitly call Rows.Close.
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews, rsc
https://golang.org/cl/79330043
|
|
R=golang-codereviews
TBR=golang-dev
CC=golang-codereviews
https://golang.org/cl/49920047
|
|
A user reported heavy contention on fmt's printer cache. Avoid
fmt.Sprint. We have to do reflection anyway, and there was
already an asString function to use strconv, so use it.
This CL also eliminates a redundant allocation + copy when
scanning into *[]byte (avoiding the intermediate string)
and avoids an extra alloc when assigning to a caller's RawBytes
(trying to reuse the caller's memory).
Fixes #7086
R=golang-codereviews, nightlyone
CC=golang-codereviews
https://golang.org/cl/50240044
|
|
R=golang-codereviews, dave, iant
CC=golang-codereviews
https://golang.org/cl/45750044
|
|
The last connection in the pool was not being handed out correctly.
R=golang-codereviews, gobot, bradfitz
CC=golang-codereviews
https://golang.org/cl/40410043
|
|
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/40370051
|
|
R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/43300043
|
|
This also fixes several connection leaks.
Fixes #5718
R=bradfitz, adg
CC=alberto.garcia.hierro, golang-dev
https://golang.org/cl/14920046
|
|
The previous coding did not correctly check for errors from the driver's
Next() or Close(), which could mask genuine errors from the database, as
witnessed in issue #6651.
Even after this change errors from Close() will be ignored if the query
returned no rows (as Rows.Next will have closed the handle already), but it
is a lot easier for the drivers to guard against that.
Fixes #6651.
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/41590043
|
|
The final condition (db.maxIdleConnsLocked() > db.freeConn.Len()) can
only be true iff db.maxIdleConnsLocked() is greater than 0, so previously
checking if it's greater than 0 is a waste, specially when that involves
a method call which (ATM) can't be inlined and includes a switch.
Dissasembly follows (test for err == nil has been omitted for clarity):
Before:
43c357: cmp $0x0,%bl
43c35a: jne 43c3ce <database/sql.(*DB).putConnDBLocked+0x1ce>
43c35c: mov %rax,(%rsp)
43c360: callq 43aec0 <database/sql.(*DB).maxIdleConnsLocked>
43c365: mov 0x8(%rsp),%rbx
43c36a: cmp $0x0,%rbx
43c36e: jle 43c3ce <database/sql.(*DB).putConnDBLocked+0x1ce>
43c370: mov 0x30(%rsp),%rbx
43c375: mov %rbx,(%rsp)
43c379: callq 43aec0 <database/sql.(*DB).maxIdleConnsLocked>
43c37e: mov 0x30(%rsp),%rdx
43c383: mov 0x8(%rsp),%rcx
43c388: mov 0x28(%rdx),%rbp
43c38c: mov 0x28(%rbp),%rbx
43c390: cmp %rcx,%rbx
43c393: jge 43c3ce <database/sql.(*DB).putConnDBLocked+0x1ce>
43c395: mov 0x28(%rdx),%rbp
43c399: mov %rbp,(%rsp)
43c39d: mov 0x38(%rsp),%rcx
43c3a2: mov $0x556c60,%eax
43c3a7: mov %rax,0x8(%rsp)
43c3ac: mov %rcx,0x10(%rsp)
43c3b1: callq 4db5b0 <container/list.(*List).PushFront>
After:
43c357: cmp $0x0,%bl
43c35a: jne 43c3b5 <database/sql.(*DB).putConnDBLocked+0x1b5>
43c35c: mov %rax,(%rsp)
43c360: callq 43aec0 <database/sql.(*DB).maxIdleConnsLocked>
43c365: mov 0x30(%rsp),%rdx
43c36a: mov 0x8(%rsp),%rcx
43c36f: mov 0x28(%rdx),%rbp
43c373: mov 0x28(%rbp),%rbx
43c377: cmp %rcx,%rbx
43c37a: jge 43c3b5 <database/sql.(*DB).putConnDBLocked+0x1b5>
43c37c: mov 0x28(%rdx),%rbp
43c380: mov %rbp,(%rsp)
43c384: mov 0x38(%rsp),%rcx
43c389: mov $0x556c60,%eax
43c38e: mov %rax,0x8(%rsp)
43c393: mov %rcx,0x10(%rsp)
43c398: callq 4db590 <container/list.(*List).PushFront>
R=golang-dev, bradfitz, iant
CC=golang-dev
https://golang.org/cl/14656044
|
|
Fixes #5110
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/19280046
|
|
R=golang-dev
CC=bradfitz, golang-dev
https://golang.org/cl/17590043
|
|
Update #5886
R=golang-dev, kamil.kisiel, adg, r, rsc, dave, arnehormann, bradfitz
CC=golang-dev
https://golang.org/cl/14087043
|
|
New test added in CL 14611045 causes a deadlock when
running the tests with -cpu=n,n because the fakedb
driver always waits when opening a new connection after
running TestConnectionLeak. Reset its state after.
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/14780043
|
|
Found by vet.
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/14762044
|
|
CL 10726044 introduced a race condition which causes connections
to be leaked under certain circumstances. If SetMaxOpenConns is
used, the application eventually deadlocks. Otherwise, the number
of open connections just keep growing indefinitely.
Fixes #6593
R=golang-dev, bradfitz, tad.glines, bketelsen
CC=golang-dev
https://golang.org/cl/14611045
|
|
Add a check at the end of every test to make sure
there are no leaked connections after running a test.
Avoid incorrectly decrementing the number of open connections
when the driver connection ends up it a bad state (numOpen was
decremented twice).
Prevent leaking a Rows struct (which ends up leaking a
connection) in Row.Scan() when a *RawBytes destination is
improperly used.
Close the Rows struct in TestRowsColumns.
Update #6593
R=golang-dev, bradfitz, dave
CC=golang-dev
https://golang.org/cl/14642044
|
|
Update #4805
Add the ability to set an open connection limit.
Fixed case where the Conn finalCloser was being called with db.mu locked.
Added separate benchmarks for each path for Exec and Query.
Replaced slice based idle pool with list based idle pool.
R=bradfitz
CC=golang-dev
https://golang.org/cl/10726044
|
|
Breaks build, and has a race.
««« original CL description
database/sql: add SetMaxOpenConns
Update #4805
Add the ability to set an open connection limit.
Fixed case where the Conn finalCloser was being called with db.mu locked.
Added seperate benchmarks for each path for Exec and Query.
Replaced slice based idle pool with list based idle pool.
R=bradfitz
CC=golang-dev
https://golang.org/cl/10726044
»»»
R=golang-dev
CC=golang-dev
https://golang.org/cl/13252046
|
|
Update #4805
Add the ability to set an open connection limit.
Fixed case where the Conn finalCloser was being called with db.mu locked.
Added seperate benchmarks for each path for Exec and Query.
Replaced slice based idle pool with list based idle pool.
R=bradfitz
CC=golang-dev
https://golang.org/cl/10726044
|
|
Rows.Close.
Previously, callers that followed the example code (but not call
rows.Close after "for rows.Next() { ... }") could leak statements if
the driver returned an error other than io.EOF.
R=bradfitz, alex.brainman
CC=golang-dev, rsc
https://golang.org/cl/12677050
|
|
R=golang-dev, kevlar, rsc, adg, r
CC=golang-dev
https://golang.org/cl/12962043
|
|
Fixes an issue where prepared statements that outlive many
connections become expensive to invoke.
Fixes #6081
R=golang-dev
CC=bradfitz, golang-dev
https://golang.org/cl/12646044
|
|
Update #6081
R=golang-dev, gri
CC=golang-dev
https://golang.org/cl/12810043
|
|
Fixes #5936
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/11620046
|
|
This should have been removed in 45c12efb4635. Not a correctness
issue, but unnecessary work.
This CL also adds paranoia checks in removeDep so this doesn't
happen again.
Fixes #5502
R=adg
CC=gobot, golang-dev, google
https://golang.org/cl/9543043
|
|
Reduces garbage.
R=adg, r
CC=dsymonds, gobot, golang-dev
https://golang.org/cl/9088045
|
|
Found while debugging memory usage. Nobody accesses this field
anymore.
R=golang-dev, i.caught.air, adg, r
CC=golang-dev
https://golang.org/cl/9108043
|
|
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/8981043
|
|
The refcounting of driver Conns was completedly busted and
would leak (be held open forever) with any reasonable
load. This was a significant regression from Go 1.0.
The core of this patch is removing one line:
s.db.addDep(dc, s)
A database conn (dc) is a resource that be re-created any time
(but cached for speed) should not be held open forever with a
dependency refcount just because the Stmt (s) is alive (which
typically last for long periods of time, like forever).
The meat of the patch is new tests. In fixing the real issue,
a lot of tests then failed due to the fakedb_test.go's paranoia
about closing a fakeConn while it has open fakeStmts on it. I
could've ignored that, but that's been a problem in the past for
other bugs.
Instead, I now track per-Conn open statements and close them
when the the conn closes. The proper way to do this would've
been making *driverStmt a finalCloser and using the dep mechanism,
but it was much more invasive. Added a TODO instead.
I'd like to give a way for drivers to opt-out of caring about
driver.Stmt closes before a driver.Conn close, but that's a TODO
for the future, and that TODO is added in this CL.
I know this is very late for Go 1.1, but database/sql is
currently nearly useless without this.
I'd like to believe all these database/sql bugs in the past
release cycle are the result of increased usage, number of
drivers, and good feedback from increasingly-capable Go
developers, and not the result of me sucking. It's also hard
with all the real drivers being out-of-tree, so I'm having to
add more and more hooks to fakedb_test.go to simulate things
which real drivers end up doing.
Fixes #5323
R=golang-dev, snaury, gwenn.kahz, google, r
CC=golang-dev
https://golang.org/cl/8836045
|
|
From the issue, which describes it as well as I could:
database/sql assumes that driver.Stmt.Close does not need the
connection.
see database/sql/sql.go:1308:
This puts the Rows' connection back into the idle pool, and
then calls the driver.Stmt.Close method of the Stmt it belongs
to. In the postgresql driver implementation
(https://github.com/lib/pq), Stmt.Close communicates with the
server (on the connection that was just put back into the idle
pool). Most of the time, this causes no problems, but if
another goroutine makes a query at the right (wrong?) time,
chaos results.
In any case, traffic is being sent on "free" connections
shortly after they are freed, leading to race conditions that
kill the driver code.
Fixes #5283
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/8633044
|
|
See https://github.com/raggi/go-and-java for runtime benchmark.
The patch reduces the amount of map key search, moving connection oriented
variables onto the connection structs.
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/8092045
|
|
Make the copy directly in the convert switch instead of an extra loop.
Also stops converting nil-[]byte to zero-[]byte when assigning to *interface
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/7962044
|
|
Fixes #5127
R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/8011044
|
|
Fixes #5046
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/8016044
|
|
E.g conversions from numeric types to RawBytes are missing, what makes RawBytes unusable in some cases.
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/7783046
|
|
R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/7865044
|
|
Update #4805
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/7634045
|
|
Now that revision 0c029965805f is in, it's easy
to guarantee that we never access a driver.Conn
concurrently, per the database/sql/driver contract,
so we can remove this overlarge mutex.
Fixes #3857
R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/7707047
|
|
The database/sql/driver docs make this promise:
"Conn is a connection to a database. It is not used
concurrently by multiple goroutines."
That promises exists as part of database/sql's overall
goal of making drivers relatively easy to write.
So far this promise has been kept without the use of locks by
being careful in the database/sql package, but sometimes too
careful. (cf. golang.org/issue/3857)
The CL associates a Mutex with each driver.Conn, and with the
interface value progeny thereof. (e.g. each driver.Tx,
driver.Stmt, driver.Rows, driver.Result, etc) Then whenever
those interface values are used, the Locker is locked.
This CL should be a no-op (aside from some new Lock/Unlock
pairs) and doesn't attempt to fix Issue 3857 or Issue 4459,
but should make it much easier in a subsequent CL.
Update #3857
R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/7803043
|
|
Fixes #4804
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/7819043
|
|
Fixes #4902
R=golang-dev, alex.brainman, r, google
CC=golang-dev
https://golang.org/cl/7579045
|
|
Return nice errors and don't panic.
Fixes #4859
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/7383046
|
|
And add a test too, for Alex. :)
Fixes #3734
R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/7399046
|
|
Simplifies the contract for Driver.Stmt.Close in
the process of fixing issue 3865.
Fixes #3865
Update #4459 (maybe fixes it; uninvestigated)
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/7363043
|
|
It used to be package "db" but was long ago renamed
to be "sql".
R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/7322075
|
|
Completly the same like the Execer-Interface, just for Queries.
This allows Drivers to execute Queries without preparing them first
R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/7085056
|