FAQ
database/sql: expose DB.MaxIdleConns and add DB.SetMaxIdleConns

The purpose of this patch is to allow for significant improvements in
concurrent
database access using the standard database/sql pool. As evidenced by
work
derived from the Boundary "Go vs. Java" blog post, the current concurent
performance is limited for certain use cases (such as net/http serving
data from
bmizerany/pq)[1]. Additional changes from the original blog post,
including
benchmarking using more modern tools is also available on GitHub[2],
some
results are included[3].

Additional notes:

* The current code defaults to 2 "idle connections". In reality this
equates to
2 "active connections" for various reasons. This default value is
continued only
such that users would not be subject to unexpected semantic changes.
* There is no maximum limit implemented. While not the subject of this
problem
or patch, this should be implemented.
* The immediate close behavior (something I am tempted to consider a
bug) has
not been altered. This point may become more of a philosophical
discussion. In
my experience, it's better to handle bursts with grace where possible.
This
would also relieve the scheduler.
* It may make sense to move the mutex in database/sql to a RW variant
with a
persistent store of Conn objects and a free / use list (with a slow
garbage
collector). This would also simplify the Close anti-race logic.
* The second comment on Open suggests that drivers will implement an
Open that
returns a *DB. This is no longer the case, and this comment should be
removed.
* The default value should be documented somewhere.

[1]
http://boundary.com/blog/2012/09/17/comparing-go-and-java-part-2-performance/
[2] https://github.com/raggi/go-and-java
[3]
https://github.com/raggi/go-and-java/blob/master/README.md#example-results

https://codereview.appspot.com/6855102/

--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Bradfitz at Mar 8, 2013 at 10:40 pm
    comments and thoughts ...



    https://codereview.appspot.com/6855102/diff/2001/src/pkg/database/sql/sql.go
    File src/pkg/database/sql/sql.go (right):

    https://codereview.appspot.com/6855102/diff/2001/src/pkg/database/sql/sql.go#newcode228
    src/pkg/database/sql/sql.go:228: // Adjust the maximum number of
    permanently retained database connections. This
    See next comment.

    // SetMaxIdleConns sets ...

    https://codereview.appspot.com/6855102/diff/2001/src/pkg/database/sql/sql.go#newcode229
    src/pkg/database/sql/sql.go:229: // defaults to 2 for historical
    reasons. N.B. This is not the maximum number of
    I think we should instead make an exported constant for 2
    (DefaultMaxIdleConns) and reference that. No need to say "for
    historical reasons". Drop the "N.B.", as good as the note is. We avoid
    e.g. i.e. n.b.

    https://codereview.appspot.com/6855102/diff/2001/src/pkg/database/sql/sql.go#newcode239
    src/pkg/database/sql/sql.go:239: // The current maximum number of idle
    database connections allowed. See
    Go comment style is to start sentences with the name of the thing being
    described. They have to stand alone.

    // MaxIdleConns returns the ....

    https://codereview.appspot.com/6855102/diff/2001/src/pkg/database/sql/sql.go#newcode242
    src/pkg/database/sql/sql.go:242: // TODO(bradfitz): ask driver, if
    supported, for its default preference
    probably not here anymore. Probably only once the driver is opened
    (where you have the 2 now).

    you should probably have a func like:

    func initialMaxIdleConns(db *DB) int {
    if di, ok := db.driver.(driver.MaxIdleConner); ok {
    if v, ok := di.MaxIdleConns(db.dsn); ok {
    return v
    }
    }
    return DefaultMaxIdleConns
    }

    and then call that from Open.

    But rather than a proliferation of small preferred-config interfaces
    like that in the driver package, perhaps we should just have an
    alternate Open interface that returns that config from the dsn once.

    Imagine, in the driver package:

    type DriverConfig struct [
    MaxIdleConns int

    // future stuff

    NewConn func() (Conn, Error)
    }

    And then we add:

    package driver
    type DriverConfig interface {
    OpenConfig(dsn string) (*DriverConfig, error)
    }

    And we prefer to use that if it exists, else falling back to
    driver.Driver's Open instead.

    That seems more extensible.

    Thoughts?

    https://codereview.appspot.com/6855102/diff/2001/src/pkg/database/sql/sql.go#newcode293
    src/pkg/database/sql/sql.go:293: maxIdle := db.MaxIdleConns()
    this is grabbing a lock and unlocking it, even though you grab the same
    lock on the next line.

    You can just use db.maxIdleConns directly once you're holding the lock.

    https://codereview.appspot.com/6855102/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedMar 8, '13 at 2:17a
activeMar 8, '13 at 10:40p
posts2
users2
websitegolang.org

2 users in discussion

Bradfitz: 1 post Raggi: 1 post

People

Translate

site design / logo © 2022 Grokbase