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

Issue 10726044: database/sql: add SetMaxOpenConns (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 4 months ago by Tad Glines
Modified:
4 years, 2 months ago
Reviewers:
bradfitz
CC:
bradfitz, golang-dev
Visibility:
Public.

Description

database/sql: add SetMaxOpenConns Update issue 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.

Patch Set 1 #

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

Total comments: 4

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

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

Total comments: 21

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

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

Total comments: 18

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

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

Total comments: 2

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+825 lines, -75 lines) Patch
M src/pkg/database/sql/fakedb_test.go View 1 4 chunks +23 lines, -5 lines 0 comments Download
M src/pkg/database/sql/sql.go View 1 2 3 4 5 6 7 14 chunks +232 lines, -44 lines 0 comments Download
M src/pkg/database/sql/sql_test.go View 1 2 3 4 5 6 7 8 12 chunks +569 lines, -25 lines 0 comments Download
M src/pkg/go/build/deps_test.go View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 24
bradfitz
https://codereview.appspot.com/10726044/diff/3001/src/pkg/database/sql/list.go File src/pkg/database/sql/list.go (right): https://codereview.appspot.com/10726044/diff/3001/src/pkg/database/sql/list.go#newcode3 src/pkg/database/sql/list.go:3: type list struct { document what this file and/or ...
4 years, 4 months ago (2013-07-02 14:30:20 UTC) #1
bradfitz
Ping. What's the status on this? Do you still want to finish it? Go 1.2 ...
4 years, 4 months ago (2013-07-18 03:50:22 UTC) #2
Tad Glines
https://codereview.appspot.com/10726044/diff/3001/src/pkg/database/sql/list.go File src/pkg/database/sql/list.go (right): https://codereview.appspot.com/10726044/diff/3001/src/pkg/database/sql/list.go#newcode3 src/pkg/database/sql/list.go:3: type list struct { On 2013/07/02 14:30:20, bradfitz wrote: ...
4 years, 4 months ago (2013-07-18 15:44:37 UTC) #3
bradfitz
I sympathize with your quest to eliminate garbage and improve performance, but can we keep ...
4 years, 3 months ago (2013-07-22 20:29:51 UTC) #4
bradfitz
Also, add to this CL's description, on its own line: Update issue 4805 On Mon, ...
4 years, 3 months ago (2013-07-22 20:31:30 UTC) #5
Tad Glines
On 2013/07/22 20:31:30, bradfitz wrote: > Also, add to this CL's description, on its own ...
4 years, 3 months ago (2013-07-22 21:29:20 UTC) #6
bradfitz
https://codereview.appspot.com/10726044/diff/17001/src/pkg/database/sql/sql.go File src/pkg/database/sql/sql.go (right): https://codereview.appspot.com/10726044/diff/17001/src/pkg/database/sql/sql.go#newcode197 src/pkg/database/sql/sql.go:197: freeConn *list.List *list.List // of type *foo https://codereview.appspot.com/10726044/diff/17001/src/pkg/database/sql/sql.go#newcode198 src/pkg/database/sql/sql.go:198: ...
4 years, 3 months ago (2013-07-22 21:48:32 UTC) #7
Tad Glines
https://codereview.appspot.com/10726044/diff/17001/src/pkg/database/sql/sql.go File src/pkg/database/sql/sql.go (right): https://codereview.appspot.com/10726044/diff/17001/src/pkg/database/sql/sql.go#newcode197 src/pkg/database/sql/sql.go:197: freeConn *list.List On 2013/07/22 21:48:33, bradfitz wrote: > *list.List ...
4 years, 3 months ago (2013-07-22 22:52:01 UTC) #8
bradfitz
Looking better. https://codereview.appspot.com/10726044/diff/31001/src/pkg/database/sql/sql.go File src/pkg/database/sql/sql.go (right): https://codereview.appspot.com/10726044/diff/31001/src/pkg/database/sql/sql.go#newcode197 src/pkg/database/sql/sql.go:197: freeConn *list.List // or driverConn // of ...
4 years, 3 months ago (2013-07-23 00:05:19 UTC) #9
Tad Glines
https://codereview.appspot.com/10726044/diff/31001/src/pkg/database/sql/sql.go File src/pkg/database/sql/sql.go (right): https://codereview.appspot.com/10726044/diff/31001/src/pkg/database/sql/sql.go#newcode197 src/pkg/database/sql/sql.go:197: freeConn *list.List // or driverConn On 2013/07/23 00:05:19, bradfitz ...
4 years, 3 months ago (2013-07-23 01:32:17 UTC) #10
bradfitz
This CL continues to scare me. I worry that it will be a maintenance nightmare.
4 years, 3 months ago (2013-08-02 17:24:27 UTC) #11
Tad Glines
On 2013/08/02 17:24:27, bradfitz wrote: > This CL continues to scare me. > > I ...
4 years, 3 months ago (2013-08-02 19:33:50 UTC) #12
bradfitz
It's just a lot of new moving pieces and state, and database/sql wasn't already the ...
4 years, 3 months ago (2013-08-20 22:05:33 UTC) #13
bradfitz
LGTM I've read this over again and am reasonably content maintaining it. Enough people have ...
4 years, 2 months ago (2013-08-30 00:19:07 UTC) #14
bradfitz
*** Submitted as https://code.google.com/p/go/source/detail?r=c9bea548fb6f *** database/sql: add SetMaxOpenConns Update issue 4805 Add the ability to ...
4 years, 2 months ago (2013-08-30 00:20:56 UTC) #15
bradfitz
But it breaks the build, so reverting. On Thu, Aug 29, 2013 at 5:20 PM, ...
4 years, 2 months ago (2013-08-30 00:26:02 UTC) #16
bradfitz
Tad: See http://build.golang.org/log/f53bbf9c888a0fe2c4a26d856c5e1449f055d287 Could you fix that? The other failure is easier to fix: deps_test.go ...
4 years, 2 months ago (2013-08-30 00:27:57 UTC) #17
Tad Glines
On 2013/08/30 00:27:57, bradfitz wrote: > Tad: > > See http://build.golang.org/log/f53bbf9c888a0fe2c4a26d856c5e1449f055d287 > > Could you ...
4 years, 2 months ago (2013-08-30 02:39:40 UTC) #18
bradfitz
https://codereview.appspot.com/10726044/diff/51001/src/pkg/database/sql/sql_test.go File src/pkg/database/sql/sql_test.go (right): https://codereview.appspot.com/10726044/diff/51001/src/pkg/database/sql/sql_test.go#newcode1222 src/pkg/database/sql/sql_test.go:1222: fini(t testing.TB) fini isn't a word or common abbreviation ...
4 years, 2 months ago (2013-08-30 02:47:51 UTC) #19
Tad Glines
https://codereview.appspot.com/10726044/diff/51001/src/pkg/database/sql/sql_test.go File src/pkg/database/sql/sql_test.go (right): https://codereview.appspot.com/10726044/diff/51001/src/pkg/database/sql/sql_test.go#newcode1222 src/pkg/database/sql/sql_test.go:1222: fini(t testing.TB) On 2013/08/30 02:47:52, bradfitz wrote: > fini ...
4 years, 2 months ago (2013-08-30 03:37:55 UTC) #20
Tad Glines
On 2013/08/30 02:39:40, Tad Glines wrote: > On 2013/08/30 00:27:57, bradfitz wrote: > > Tad: ...
4 years, 2 months ago (2013-08-30 03:43:01 UTC) #21
bradfitz
The codereview tool handles the Author stuff. Btw, drop the parens from your subject. See ...
4 years, 2 months ago (2013-08-30 03:44:33 UTC) #22
Tad Glines
On 2013/08/30 03:44:33, bradfitz wrote: > The codereview tool handles the Author stuff. > > ...
4 years, 2 months ago (2013-08-30 04:32:11 UTC) #23
bradfitz
4 years, 2 months ago (2013-08-30 16:27:38 UTC) #24
*** Submitted as https://code.google.com/p/go/source/detail?r=bf9de8d2e308 ***

database/sql: add SetMaxOpenConns

Update issue 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://codereview.appspot.com/10726044

Committer: Brad Fitzpatrick <bradfitz@golang.org>
Sign in to reply to this message.

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