In function connectionCleanerRunLocked append to closing slice affects db.freeConns and vise versa. Sometimes valid connections are closed and some invalid not.
Change-Id: I5282f15be3e549533b7d994b17b2060db3c0e7da
GitHub-Last-Rev: b3eb3ab6f4
GitHub-Pull-Request: golang/go#49429
Reviewed-on: https://go-review.googlesource.com/c/go/+/362214
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When drivers return driver.ErrBadConn, no meaningful
information about what the cause of the problem is
returned. Ideally the driver.ErrBadConn would be
always caught with the retry loop, but this is not
always the case. Drivers today must choose between
returning a useful error and use the rety logic.
This allows supporting both.
Fixes#47142
Change-Id: I454573028f041dfdf874eed6c254fb194ccf6d96
Reviewed-on: https://go-review.googlesource.com/c/go/+/333949
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Trust: Ian Lance Taylor <iant@golang.org>
Trust: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fix idle connection reuse so that ConnMaxIdleTime clears down excessive
idle connections.
This now ensures that db.freeConn is ordered by returnedAt and that
connections that have been idle for the shortest period are reused
first.
In addition connectionCleanerRunLocked updates the next check deadline
based on idle and maximum life time information so that we avoid waiting
up to double MaxIdleTime to close connections.
Corrected the calling timer of connectionCleaner.
Fixes#39471
Change-Id: I6d26b3542179ef35aa13e5265a89bc0f08ba7fa1
Reviewed-on: https://go-review.googlesource.com/c/go/+/237337
Reviewed-by: Tamás Gulácsi <tgulacsi78@gmail.com>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Ian Lance Taylor <iant@golang.org>
Change-Id: I519b8021fa79dccc5c0ee79134547491116fc4cc
GitHub-Last-Rev: 48869f5434
GitHub-Pull-Request: golang/go#48071
Reviewed-on: https://go-review.googlesource.com/c/go/+/346231
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Keith Randall <khr@golang.org>
The issue go#46783 correctly diagnosed the context timeout
caused an intermittent failure when the context was canceled
prior to the BeginTx call. However due to the asynchronous nature
of canceling a Tx through a context on fast systems, the tx.Prepare
also succeeded. On slower systems or if a time.Sleep was inserted
between the BeginTx and Prepare, the Prepare would fail.
Resolve this by moving the context cancel after the Prepare.
This will still trigger the deadlock which I tested locally.
In addition, I interspersed multiple time.Sleep calls and the
test still functioned.
Fixes#46852
Change-Id: I9cbf90d3c12b2555493a37799738772b615ae39d
Reviewed-on: https://go-review.googlesource.com/c/go/+/329830
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>
This follows the spelling choices that the Go project has made for English words.
https://github.com/golang/go/wiki/Spelling
Change-Id: Ie7c586d2cf23020cb492cfff58c0831d2d8d3a78
GitHub-Last-Rev: e16a32cd22
GitHub-Pull-Request: golang/go#45442
Reviewed-on: https://go-review.googlesource.com/c/go/+/308291
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
The Go standard library retrofitted context support onto existing APIs
using context.Background and later offered variants that directly
supported user-defined context value specification. This commit makes
that behavior clear in documentation and suggests context-aware
alternatives if the user is looking for one.
An example motivation is supporting code for use in systems that expect
APIs to be cancelable for lifecycle correctness or load
shedding/management reasons, as alluded to in
https://blog.golang.org/context-and-structs.
Updates #44143
Change-Id: I2d7f954ddf9b48264d5ebc8d0007058ff9bddf14
Reviewed-on: https://go-review.googlesource.com/c/go/+/296152
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Jean de Klerk <deklerk@google.com>
Trust: Jean de Klerk <deklerk@google.com>
Run-TryBot: Jean de Klerk <deklerk@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This change allows driver implementations to manage resources in
driver.Connector, e.g. to share the same underlying database handle
between multiple connections. That is, it allows embedded databases
with in-memory backends like SQLite and Genji to safely release the
resources once the sql.DB is closed.
This makes it possible to address oddities with in-memory stores in
SQLite and Genji drivers without introducing too much complexity in
the driver implementations.
See also:
- https://github.com/mattn/go-sqlite3/issues/204
- https://github.com/mattn/go-sqlite3/issues/511
- https://github.com/genjidb/genji/issues/210Fixes#41790
Change-Id: Idbd19763134438ed38288b9d44f16608e4e97fd7
GitHub-Last-Rev: 962c785dfb
GitHub-Pull-Request: golang/go#41710
Reviewed-on: https://go-review.googlesource.com/c/go/+/258360
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
This test contained a data race.
On line 437, db.BeginTx starts a goroutine that runs tx.awaitDone,
which reads tx.keepConnOnRollback.
On line 445, the test writes to tx.keepConnOnRollback.
tx.awaitDone waits on ctx, but because ctx is timeout-based,
there's no ordering guarantee between the write and the read.
The race detector never caught this before
because the context package implementation of Done
contained enough synchronization to make it safe.
That synchronization is not package of the context API or guarantees,
and the first several releases it was not present.
Another commit soon will remove that synchronization,
exposing the latent data race.
To fix the race, emulate a time-based context
using an explicit cancellation-based context.
This gives us enough control to avoid the race.
Change-Id: I103fe9b987b1d4c02e7a20ac3c22a682652128b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/288493
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Found by go vet pass "testinggoroutines".
Change-Id: I6360af2079617b7aa62dcb9bd7254578ca5d1c1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/235527
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Drop error check because errors can be
not only ErrTxDone for tx stmt executions,
and the purpose of the test is just reproducing
deadlock.
Fixes#42259
Change-Id: I9e7105ada1403ec7064dcc1c3385b36893a1c195
Reviewed-on: https://go-review.googlesource.com/c/go/+/266097
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Tx acquires tx.closemu W-lock and then acquires stmt.closemu.W-lock
to fully close the transaction and associated prepared statement.
Stmt query and execution run in reverse ways - acquires
stmt.closemu.R-lock and then acquires tx.closemu.R-lock to grab tx
connection, which may cause deadlock.
Prevent the lock is held around tx.closePrepared to ensure no
deadlock happens.
Fixes#40985
Change-Id: If53909822b87bce11861a6e3035ecb9476d2cd17
Reviewed-on: https://go-review.googlesource.com/c/go/+/250178
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Previously, 2 goroutines were created in OpenDB and a comment in the DB.close() field indicated that they were canceled. Later, session Resetter () was removed, but the comment remained the same. This commit just fixes this message
Change-Id: Ie81026f51d7770e9cf8004818154021f626fb2e8
GitHub-Last-Rev: 38b338a0d1
GitHub-Pull-Request: golang/go#42191
Reviewed-on: https://go-review.googlesource.com/c/go/+/264838
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
When zero or less, maxIdleTime and maxLifetime means unlimited.
Helper function shortestIdleTimeLocked must not return the
minimum of the two until both are verified to be greater
then zero.
Fixes#40841
Change-Id: I1130332baf4ad259cd90c10f4221f5def8510655
Reviewed-on: https://go-review.googlesource.com/c/go/+/248817
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
The prior implementation used the format verb %v which unfortunately
improperly wrapped any underlying scanner errors, and we couldn't use
errors.Is nor errors.As. This change fixes that by using the %w verb.
Added a unit to ensure that both error sub string matching works, but
also that errors.Is works as expected.
Fixes#38099
Change-Id: Iea667041dd8081d961246f77f2542330417292dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/248337
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Opening a connection with Connect should still create a derived
context with a timeout because some clients will not use a timeout
and the connection pool may open a connection asynchronously.
Likewise, if a connection close makes a network operation it should
provide some type of sane timeout for the operation.
Fixes#38185
Change-Id: I9b7ce2996c81c486170dcc84b12672a99610fa27
Reviewed-on: https://go-review.googlesource.com/c/go/+/230438
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Do not cancel rows during test. Only cancel the Tx.
Correct the referenced issue number on the test.
Fixes#38597
Change-Id: I0e8ba1bf2a8ba638d121c9c6938501fec1d5e961
Reviewed-on: https://go-review.googlesource.com/c/go/+/229478
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously the connection pool would only count connections
expired in the background connectionCleaner goroutine towards the
MaxLifetimeClosed stat.
This change increments the stat correctly when checking for
expiry in when acquiring and releasing a connection.
Fixes#38058
Change-Id: Id707ddd40a42a4c38658d5f2931da131647d6c29
GitHub-Last-Rev: 0f205ede43
GitHub-Pull-Request: golang/go#38263
Reviewed-on: https://go-review.googlesource.com/c/go/+/227278
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Previously the Tx would drop the connection after rolling back from
a context cancel. Now if the driver can reset the session,
keep the connection.
Change-Id: Ie6a3124275632787629844d91a06bb2e70cc060b
Reviewed-on: https://go-review.googlesource.com/c/go/+/216241
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It was possible for a Tx that was aborted for rollback
asynchronously to execute a query after the rollback had completed
on the database, which often would auto commit the query outside
of the transaction.
By W-locking the tx.closemu prior to issuing the rollback
connection it ensures any Tx query either fails or finishes
on the Tx, and never after the Tx has rolled back.
Fixes#34775Fixes#32942
Change-Id: I017b7932082f2f4ead70bae08b61ed9068ac1d01
Reviewed-on: https://go-review.googlesource.com/c/go/+/216240
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
With the original connection reuse strategy, it was possible that
when a new connection was requested, the pool would wait for an
an existing connection to return for re-use in a full connection
pool, and then it would check if the returned connection was expired.
If the returned connection expired while awaiting re-use, it would
return an error to the location requestiong the new connection.
The existing call sites requesting a new connection was often the last
attempt at returning a connection for a query. This would then
result in a failed query.
This change ensures that we perform the expiry check right
before a connection is inserted back in to the connection pool
for while requesting a new connection. If requesting a new connection
it will no longer fail due to the connection expiring.
Fixes#32530
Change-Id: If16379befe0e14d90160219c0c9396243fe062f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/216197
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This addresses comments made by Russ after
https://golang.org/cl/174122 was merged. It addes a test
for the connection validator and renames the interface to just
"Validator".
Change-Id: Iea53e9b250c9be2e86e9b75906e7353e26437c5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/223963
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
The Row.Err method is intended to assist wrapping sql.DB.
Because sql.Row is a struct with private fields,
a wrapper in an existing code base cannot easily provide users
with a different implementation without large rewrites.
Adding this method allows query level errors to be handled
centrally.
Fixes#35804
Change-Id: I94e6329de89a7ee1284ce9ef76af4363d2d081f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/214317
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It's a very minor error, but it's a bad copy/paste example.
Change-Id: Ia6a723c31f2205c933857ce2cf715bddf773ebb6
GitHub-Last-Rev: 7f14b1a5c1
GitHub-Pull-Request: golang/go#37932
Reviewed-on: https://go-review.googlesource.com/c/go/+/223960
Reviewed-by: Daniel Theophanes <kardianos@gmail.com>
Adds a new interface, driver.ConnectionValidator, to allow
drivers to signal they should not be used again,
separatly from the session resetter interface.
This is done now that the session reset is done
after the connection is put into the connection pool.
Previous behavior attempted to run Session Resets
in a background worker. This implementation had two
problems: untested performance gains for additional
complexity, and failures when the pool size
exceeded the connection reset channel buffer size.
Fixes#31480
Change-Id: I7d483b883c24a362c292471e87a88db5b204d1d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/174122
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Allow removing a connection from the connection pool after
it has been idle for a period of time, without regard to the
total lifespan of the connection.
Fixes#25232
Change-Id: Icff157b906769a2d2d45c67525e04a72feb8d832
Reviewed-on: https://go-review.googlesource.com/c/go/+/145758
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Found by running the go vet pass 'testinggoroutine' that
I started in CL 212920.
Change-Id: Ic9462fac85dbafc437fe4a323b886755a67a1efa
Reviewed-on: https://go-review.googlesource.com/c/go/+/213097
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This var _ = log.Printf line was added 8 years ago, in CL 4973055,
which created the database/sql package and its tests. There was no
goimports back then, so this was likely added to make it easier to
use log package during development of tests.
It's no longer needed, so remove it. It can always be conveniently
re-added via goimports whenever needed.
Change-Id: I7c32ae3e593c194d970920084139dfa5a42386dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/202481
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The required slice capacity is already known. Thus, preallocate a slice with the correct capacity before appending to it.
Change-Id: I45ac2c5f1701caeb3dda20451d371713ae7e7365
GitHub-Last-Rev: 2bf575be65
GitHub-Pull-Request: golang/go#34602
Reviewed-on: https://go-review.googlesource.com/c/go/+/197917
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Use the following (suboptimal) script to obtain a list of possible
typos:
#!/usr/bin/env sh
set -x
git ls-files |\
grep -e '\.\(c\|cc\|go\)$' |\
xargs -n 1\
awk\
'/\/\// { gsub(/.*\/\//, ""); print; } /\/\*/, /\*\// { gsub(/.*\/\*/, ""); gsub(/\*\/.*/, ""); }' |\
hunspell -d en_US -l |\
grep '^[[:upper:]]\{0,1\}[[:lower:]]\{1,\}$' |\
grep -v -e '^.\{1,4\}$' -e '^.\{16,\}$' |\
sort -f |\
uniq -c |\
awk '$1 == 1 { print $2; }'
Then, go through the results manually and fix the most obvious typos in
the non-vendored code.
Change-Id: I3cb5830a176850e1a0584b8a40b47bde7b260eae
Reviewed-on: https://go-review.googlesource.com/c/go/+/193848
Reviewed-by: Robert Griesemer <gri@golang.org>
Some were never used, and some haven't been used for years.
One exception is net/http's readerAndCloser, which was only used in a
test. Move it to a test file.
While at it, remove a check in regexp that could never fire; the field
is an uint32, so it can never be negative.
Change-Id: Ia2200f6afa106bae4034045ea8233b452f38747b
Reviewed-on: https://go-review.googlesource.com/c/go/+/192621
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add support for scanning decimal types into values. If the dest
supports the decimal composer interface and the src supports
the decimal decomposer, set the value of the decimal when Scanning.
Add support for sending decimal decomposer interface values
as parameters.
For #30870
Change-Id: Ic5dbf9069df8d56405852b17542a9188d55c2947
Reviewed-on: https://go-review.googlesource.com/c/go/+/174181
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Exposing the underlying driver conn will allow the use of the
standard connection pool while still able to run special function
directly on the driver.
Fixes#29835
Change-Id: Ib6d3b9535e730f008916805ae3bf76e4494c88f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/174182
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
While it is possible for the connection pool to guard against panics
on every valuer read, this seems like a high cost to be added,
in both code complexity and possible runtime performance.
Most uses of the Valuer will be trivial, like returning
a struct field. Optimize for that case. If sometime may panic the
valuer should itself use recover and return an error.
Fixes#26332
Change-Id: Iad18780b8028f669f5a7841b74a5384d62fb6a7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/170700
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Most changes are removing redundant declaration of type when direct
instantiating value of map or slice, e.g. []T{T{}} become []T{{}}.
Small changes are removing the high order of subslice if its value
is the length of slice itself, e.g. T[:len(T)] become T[:].
The following file is excluded due to incompatibility with go1.4,
- src/cmd/compile/internal/gc/ssa.go
Change-Id: Id3abb09401795ce1e6da591a89749cba8502fb26
Reviewed-on: https://go-review.googlesource.com/c/go/+/166437
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It is common for database integers to be represented as int32
internally. Although NullInt64 is already defined,
this should remove some type casts and make working with those eaiser.
For #31231
Change-Id: Ia0c37ecef035fee0734c1d1fb6f58aef6905cf5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/174178
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
A nil src (NULL database value) will result in a "nil" string,
which will never parse correctly in a ParseInt or similar
numeric conversion. The resulting error is confusing. Check
for a nil src prior to converting the value to string
if the resulting string will be parsed after that.
Closes#31274
Change-Id: I90f12cceff00fbbfdd3e343b04fa7e2596390e6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/174177
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Though there is variation in the spelling of canceled,
cancellation is always spelled with a double l.
Reference: https://www.grammarly.com/blog/canceled-vs-cancelled/
Change-Id: I240f1a297776c8e27e74f3eca566d2bc4c856f2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/170060
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Pavel
Mostafa Solati
Daniel Theophanes
Steven Hartland
Cuong Manh Le
Alexander Melentyev
korzhao
Ariel Mashraki
Naman Gera
Elias506
Matt T. Proud
Ivan Trubach
Josh Bleecher Snyder
Hein Khant Zaw
Emmanuel T Odeke
Tzu-Chiao Yeh
Ayzat Sadykov
Shinnosuke Sawada
Tim Möhlmann
Brad Erickson
Kevin Burke
Vivian Liang
Colin
Harmen
Dmitri Shuralyov
Julien Schmidt
Ainar Garipov
Daniel Martí
Brad Fitzpatrick
Shulhan