Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(958)

Issue 8797045: database/sql: move pooling logic into seperate plugable...

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 11 months ago by Tad Glines
Modified:
10 years, 10 months ago
Visibility:
Public.

Description

database/sql: move pooling logic into seperate plugable interface, added ability to chain drivers. Added the following interfaces: ConnChecker ChainableDriver ChainedDriver PoolableDriver Pool PooledStmt Add new function OpenChain that allows creation of a driver chain and/or configuration of the default pool. Default PoolableDriver's Pool implementation includes the following optional capabilities: - Limit number of connections - Maintain a minimum number of connections - Opening new connections in mulitples - Close idle connections after a specified timeout (default is immediately) - Check the connection before returning it from the pool. - Check all idle connections periodically - Use a custom query to check the connection (instead of the ConnChecker interface) - Use the new ErrDBLimitExceeded (if returned by driver) to scale back the connection limit - Configure the number of times to try acquiring a new connection before considering the DB dead and stoping all connection attempts - Configure the delay before retrying a failed connection attempt. - Configure an acquisition timeout. If set, the pool will return ErrOpenTimeout if it could not return a connection from the pool before the timeout.

Patch Set 1 #

Patch Set 2 : diff -r 5e23075b0970 https://code.google.com/p/go #

Patch Set 3 : diff -r 5e23075b0970 https://code.google.com/p/go #

Total comments: 6

Patch Set 4 : diff -r 5e23075b0970 https://code.google.com/p/go #

Total comments: 10

Patch Set 5 : diff -r 5e23075b0970 https://code.google.com/p/go #

Patch Set 6 : diff -r 30c566874b83 https://code.google.com/p/go #

Patch Set 7 : diff -r 30c566874b83 https://code.google.com/p/go #

Patch Set 8 : diff -r d29da2ced72b https://code.google.com/p/go #

Patch Set 9 : diff -r d29da2ced72b https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3804 lines, -1009 lines) Patch
M src/pkg/database/sql/convert.go View 1 2 chunks +1 line, -7 lines 0 comments Download
M src/pkg/database/sql/driver/driver.go View 1 2 3 4 5 7 chunks +173 lines, -6 lines 0 comments Download
M src/pkg/database/sql/fakedb_test.go View 1 2 3 4 5 6 7 8 26 chunks +204 lines, -21 lines 0 comments Download
A src/pkg/database/sql/list.go View 1 2 3 4 5 6 7 1 chunk +122 lines, -0 lines 0 comments Download
A src/pkg/database/sql/list_test.go View 1 2 3 4 5 6 7 1 chunk +50 lines, -0 lines 0 comments Download
A src/pkg/database/sql/pool.go View 1 2 3 4 5 6 1 chunk +1607 lines, -0 lines 0 comments Download
M src/pkg/database/sql/sql.go View 1 2 3 4 5 22 chunks +604 lines, -871 lines 0 comments Download
M src/pkg/database/sql/sql_test.go View 1 2 3 4 5 6 7 8 18 chunks +1043 lines, -104 lines 0 comments Download

Messages

Total messages: 23
Tad Glines
I just ran "go test -bench ." and there are some concurrency issues I was ...
10 years, 11 months ago (2013-04-27 00:33:07 UTC) #1
Tad Glines
changeset 3 fixed the concurrency issue. Also, related to this CL as a whole: This ...
10 years, 11 months ago (2013-04-27 01:34:27 UTC) #2
0xjnml
On 2013/04/27 01:34:27, Tad Glines wrote: I cannot recall this CL being discussed at the ...
10 years, 11 months ago (2013-04-27 07:58:14 UTC) #3
julienschmidt
I have not reviewed your complete code yet, these are just a few first thoughts. ...
10 years, 11 months ago (2013-04-27 13:09:12 UTC) #4
Tad Glines
On 2013/04/27 07:58:14, 0xjnml wrote: > On 2013/04/27 01:34:27, Tad Glines wrote: > > I ...
10 years, 11 months ago (2013-04-27 14:49:43 UTC) #5
Tad Glines
On 2013/04/27 13:09:12, julienschmidt wrote: > I have not reviewed your complete code yet, these ...
10 years, 11 months ago (2013-04-27 15:03:43 UTC) #6
ioe
Just some comments about error handling. Another thing I noted: You RLock/RUnlock the database in ...
10 years, 11 months ago (2013-04-27 23:50:40 UTC) #7
Tad Glines
Patch Set 4 addresses some comments (I'll reply to those individually) and fixes some additional ...
10 years, 11 months ago (2013-04-28 00:57:14 UTC) #8
Tad Glines
On 2013/04/27 13:09:12, julienschmidt wrote: > https://codereview.appspot.com/8797045/diff/6001/src/pkg/database/sql/pool.go#newcode149 > src/pkg/database/sql/pool.go:149: } else if c, ok := ...
10 years, 11 months ago (2013-04-28 00:58:14 UTC) #9
Tad Glines
On 2013/04/27 23:50:40, ioe wrote: > Just some comments about error handling. > > Another ...
10 years, 11 months ago (2013-04-28 01:14:56 UTC) #10
tux21b
Hi Tad, many thanks for this CL. I was already waiting eagerly for the abstraction ...
10 years, 11 months ago (2013-04-28 08:48:49 UTC) #11
Tad Glines
> many thanks for this CL. I was already waiting eagerly for the abstraction of ...
10 years, 11 months ago (2013-04-28 15:20:47 UTC) #12
ioe
Hi Tad, found some timing related nits. TL;DR: I suggest to use use time.Duration for ...
10 years, 11 months ago (2013-05-05 21:27:21 UTC) #13
Tad Glines
All time durations in the config are now of type time.Duration. https://codereview.appspot.com/8797045/diff/18001/src/pkg/database/sql/pool.go File src/pkg/database/sql/pool.go (right): ...
10 years, 10 months ago (2013-05-07 01:23:47 UTC) #14
Tad Glines
I've uploaded patch set 5. This patch set includes: - Changes to address comments by ...
10 years, 10 months ago (2013-05-07 01:34:00 UTC) #15
Tad Glines
Patch set 6 includes the following: - Re-added TestStmtCloseDeps and fixed the issues found by ...
10 years, 10 months ago (2013-05-13 00:21:48 UTC) #16
Tad Glines
Patch set 8 include the list implementation files that I forgot to include in patch ...
10 years, 10 months ago (2013-05-13 13:10:09 UTC) #17
bradfitz
This is too big to go in as-is. I think it's even too big to ...
10 years, 10 months ago (2013-05-22 01:31:30 UTC) #18
Tad Glines
On 2013/05/22 01:31:30, bradfitz wrote: > This is too big to go in as-is. Julien ...
10 years, 10 months ago (2013-05-22 02:54:14 UTC) #19
bradfitz
Sorry for the slow response. I'm totally cool with adding connection limit logic to database/sql. ...
10 years, 10 months ago (2013-05-25 16:57:29 UTC) #20
Tad Glines
On 2013/05/25 16:57:29, bradfitz wrote: > Sorry for the slow response. I assumed you are ...
10 years, 10 months ago (2013-05-25 17:06:28 UTC) #21
bradfitz
On Sat, May 25, 2013 at 10:06 AM, <Tad.Glines@gmail.com> wrote: > On 2013/05/25 16:57:29, bradfitz ...
10 years, 10 months ago (2013-05-25 18:07:34 UTC) #22
Tad Glines
10 years, 10 months ago (2013-05-30 17:03:23 UTC) #23
On 2013/05/25 16:57:29, bradfitz wrote:
> I'm totally cool with adding connection limit logic to database/sql.

The CL for this is here: https://codereview.appspot.com/9789044/
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b