New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

database/sql: set maximum lifetime of connection #9851

Closed
methane opened this Issue Feb 12, 2015 · 24 comments

Comments

Projects
None yet
@methane
Contributor

methane commented Feb 12, 2015

MySQL has wait timeout option. It limits sum of idle time.
If wait_timeout=3600, client should not use connection created before one hour ago.
Otherwise, connection may be killed while transaction.

Additionally, MySQL's global configuration is applied to only new connections.
So I want control how long client reuse connection. But sql.DB doesn't support limiting connection's lifetime.

May I send patch that adds DB.SetMaxReuseTimeout(time.Duration)?
Is there any plan to add more general way to customize behavior of connection pool?
(This may be related to #4805)

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Feb 12, 2015

It would be great to see this!

parkr commented Feb 12, 2015

It would be great to see this!

@nightlyone

This comment has been minimized.

Show comment
Hide comment
@nightlyone

nightlyone Feb 12, 2015

Contributor

An alternative would be to add an optional interface to database/sql/driver for checking liveness of a connection. That would give the driver more constraints and allow the go std library more freedom when to actually check this (e.g. via small batches or on prepare).

Contributor

nightlyone commented Feb 12, 2015

An alternative would be to add an optional interface to database/sql/driver for checking liveness of a connection. That would give the driver more constraints and allow the go std library more freedom when to actually check this (e.g. via small batches or on prepare).

@johto

This comment has been minimized.

Show comment
Hide comment
@johto

johto Feb 12, 2015

Contributor

I'm not saying this is a good solution, but as a workaround, couldn't you just return ErrBadConn from the driver if you notice the connection has been sitting idle for a long time?

Contributor

johto commented Feb 12, 2015

I'm not saying this is a good solution, but as a workaround, couldn't you just return ErrBadConn from the driver if you notice the connection has been sitting idle for a long time?

@methane

This comment has been minimized.

Show comment
Hide comment
@methane

methane Feb 13, 2015

Contributor

@johto Yes, I can. But it should be done carefully since hardcoded const maxBadConnRetries = 10 in database/sql.

Contributor

methane commented Feb 13, 2015

@johto Yes, I can. But it should be done carefully since hardcoded const maxBadConnRetries = 10 in database/sql.

@johto

This comment has been minimized.

Show comment
Hide comment
@johto

johto Feb 13, 2015

Contributor

Yes, I can. But it should be done carefully since hardcoded const maxBadConnRetries = 10 in database/sql.

Yeah, but if the conn is known dead anyways, the behavior is ultimately the same.

Contributor

johto commented Feb 13, 2015

Yes, I can. But it should be done carefully since hardcoded const maxBadConnRetries = 10 in database/sql.

Yeah, but if the conn is known dead anyways, the behavior is ultimately the same.

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Feb 13, 2015

Member

I'm thinking, sql.DB are hiding connection of driver intentionally. So RDBMS-specific problems should be resolved in the driver-side interface.
For example, go-sqlite3 provide way to hook and get driver-native connection like below.

https://github.com/mattn/go-sqlite3/blob/master/_example/hook/hook.go#L13-L18

This is not beautiful but works pretty good.

Member

mattn commented Feb 13, 2015

I'm thinking, sql.DB are hiding connection of driver intentionally. So RDBMS-specific problems should be resolved in the driver-side interface.
For example, go-sqlite3 provide way to hook and get driver-native connection like below.

https://github.com/mattn/go-sqlite3/blob/master/_example/hook/hook.go#L13-L18

This is not beautiful but works pretty good.

@johto

This comment has been minimized.

Show comment
Hide comment
@johto

johto Feb 13, 2015

Contributor

I'm thinking, sql.DB are hiding connection of driver intentionally. So RDBMS-specific problems should be resolved in the driver-side interface.

Problems? Yeah, probably. But enforcing connection lifetimes isn't a database-specific problem.

For example, go-sqlite3 provide way to hook and get driver-native connection like below.

This, to me, seems ugly beyond any recognition. For example, how do you make sure that the connection isn't handed out by database/sql while you're working on the underlying driver-specific handle?

I think the proper solution would start out something like this: https://groups.google.com/d/topic/golang-dev/RWmOv4SYUmc/discussion

Contributor

johto commented Feb 13, 2015

I'm thinking, sql.DB are hiding connection of driver intentionally. So RDBMS-specific problems should be resolved in the driver-side interface.

Problems? Yeah, probably. But enforcing connection lifetimes isn't a database-specific problem.

For example, go-sqlite3 provide way to hook and get driver-native connection like below.

This, to me, seems ugly beyond any recognition. For example, how do you make sure that the connection isn't handed out by database/sql while you're working on the underlying driver-specific handle?

I think the proper solution would start out something like this: https://groups.google.com/d/topic/golang-dev/RWmOv4SYUmc/discussion

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Feb 13, 2015

Member

You can't do it because go's driver have connection pool. So we can't know which connection is working for the current query.

Member

mattn commented Feb 13, 2015

You can't do it because go's driver have connection pool. So we can't know which connection is working for the current query.

@johto

This comment has been minimized.

Show comment
Hide comment
@johto

johto Feb 13, 2015

Contributor

You can't do it because go's driver have connection pool. So we can't know which connection is working for the current query.

Can't do what exactly? And that's my point; we need something to pull a connection out of the pool, and then expose the driver interface beneath that connection.

Contributor

johto commented Feb 13, 2015

You can't do it because go's driver have connection pool. So we can't know which connection is working for the current query.

Can't do what exactly? And that's my point; we need something to pull a connection out of the pool, and then expose the driver interface beneath that connection.

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Feb 13, 2015

Member

Can't do what exactly? And that's my point; we need something to pull a connection out of the pool, and then expose the driver interface beneath that connection.

Yes, go hides native-driver interfaces to support any RDBCS, and to do not depend on the specific DB, I think.

Member

mattn commented Feb 13, 2015

Can't do what exactly? And that's my point; we need something to pull a connection out of the pool, and then expose the driver interface beneath that connection.

Yes, go hides native-driver interfaces to support any RDBCS, and to do not depend on the specific DB, I think.

@methane

This comment has been minimized.

Show comment
Hide comment
@methane

methane Feb 13, 2015

Contributor

@mattn It's hard to clean expired connections from freelist in DB from driver's side.

I don't know other RDBs have problems similar to MySQL or not.
But it may be valuable for other RDBs to reduce connections when application is idle.
So I think this is not RDBMS-specific issue.

Contributor

methane commented Feb 13, 2015

@mattn It's hard to clean expired connections from freelist in DB from driver's side.

I don't know other RDBs have problems similar to MySQL or not.
But it may be valuable for other RDBs to reduce connections when application is idle.
So I think this is not RDBMS-specific issue.

@johto

This comment has been minimized.

Show comment
Hide comment
@johto

johto Feb 13, 2015

Contributor

But it may be valuable for other RDBs to reduce connections when application is idle.
So I think this is not RDBMS-specific issue.

Yeah, exactly. Essentially, the gap between MaxOpenConns and MaxIdleConns right now is closed immediately if the connections go idle, i.e. there's a timeout of 0. What I'd like to see is something which only starts closing idle connections after a non-zero timeout. So if you want to enforce that no client is kept around idle for more than one minute, you could do:

  db.SetMaxIdleConns(0)
  db.SetMaxReuseTimeout(time.Minute)
  // perhaps set MaxOpenConns to whatever

If the reuse timeout defaults to 0, the interface stays backwards compatible in a nice way.

Contributor

johto commented Feb 13, 2015

But it may be valuable for other RDBs to reduce connections when application is idle.
So I think this is not RDBMS-specific issue.

Yeah, exactly. Essentially, the gap between MaxOpenConns and MaxIdleConns right now is closed immediately if the connections go idle, i.e. there's a timeout of 0. What I'd like to see is something which only starts closing idle connections after a non-zero timeout. So if you want to enforce that no client is kept around idle for more than one minute, you could do:

  db.SetMaxIdleConns(0)
  db.SetMaxReuseTimeout(time.Minute)
  // perhaps set MaxOpenConns to whatever

If the reuse timeout defaults to 0, the interface stays backwards compatible in a nice way.

@methane

This comment has been minimized.

Show comment
Hide comment
@methane

methane Feb 17, 2015

Contributor

Here is rough draft: https://gist.github.com/methane/6e77daac5a8c205ac359

There are some points remaining that I should decide before post it to gerrit.

  • Time precision: seconds is OK?
    • If it's OK, should SetMaxReuseTimeout() accept int64 instead of time.Duration?
  • Can I use monotonic clock?
  • How can I write test?
Contributor

methane commented Feb 17, 2015

Here is rough draft: https://gist.github.com/methane/6e77daac5a8c205ac359

There are some points remaining that I should decide before post it to gerrit.

  • Time precision: seconds is OK?
    • If it's OK, should SetMaxReuseTimeout() accept int64 instead of time.Duration?
  • Can I use monotonic clock?
  • How can I write test?
@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Feb 18, 2015

Member

Ah, you are talking about timeout that is implemented on go. I was confused something what mysql-lib doing that. sorry.

Member

mattn commented Feb 18, 2015

Ah, you are talking about timeout that is implemented on go. I was confused something what mysql-lib doing that. sorry.

@methane

This comment has been minimized.

Show comment
Hide comment
@methane

methane Mar 3, 2015

Contributor

I've send patch to gerrint; https://go-review.googlesource.com/6580
But I want it is more clean and testable...

Contributor

methane commented Mar 3, 2015

I've send patch to gerrint; https://go-review.googlesource.com/6580
But I want it is more clean and testable...

@rsc rsc changed the title from database/sql: Add ability to set maximum lifetime of connection to database/sql: set maximum lifetime of connection Apr 10, 2015

@rsc rsc added this to the Go1.5Maybe milestone Apr 10, 2015

@gopherbot

This comment has been minimized.

Show comment
Hide comment
@gopherbot

gopherbot commented Apr 25, 2015

CL https://golang.org/cl/6580 mentions this issue.

@rsc rsc modified the milestones: Go1.6Early, Go1.5Maybe Jul 14, 2015

@ChrisHines

This comment has been minimized.

Show comment
Hide comment
@ChrisHines

ChrisHines Sep 16, 2015

Contributor

@methane Are we sure that this feature is doing the right thing? The linked MySQL docs for wait_timeout say:

The number of seconds the server waits for activity on a noninteractive connection before closing it.

The docs for the MySQL server has gone away error use similar language.

By default, the server closes the connection after eight hours if nothing has happened.

Perhaps not authoritative, but certainly more explicit, this MySQL DBA's blog says (near the bottom):

The timeout is reset each time a query is run...

All of the evidence I can find indicates that MySQL closes connections that have not run a query for a number of seconds. I can find no evidence that the timeout is measured from when the connection was created as the initial feature request and CL 6580 describe.

Contributor

ChrisHines commented Sep 16, 2015

@methane Are we sure that this feature is doing the right thing? The linked MySQL docs for wait_timeout say:

The number of seconds the server waits for activity on a noninteractive connection before closing it.

The docs for the MySQL server has gone away error use similar language.

By default, the server closes the connection after eight hours if nothing has happened.

Perhaps not authoritative, but certainly more explicit, this MySQL DBA's blog says (near the bottom):

The timeout is reset each time a query is run...

All of the evidence I can find indicates that MySQL closes connections that have not run a query for a number of seconds. I can find no evidence that the timeout is measured from when the connection was created as the initial feature request and CL 6580 describe.

@methane

This comment has been minimized.

Show comment
Hide comment
@methane

methane Sep 16, 2015

Contributor

I'm sorry. I was wrong about wait_timeout.

But still there are some cases old connections should be killed.

  • setting global variable (e.g. set @@global.query_cache_type=0).
  • When adding new read-only slave to load balancer, or gracefully switching slave.

Killing not recently used connections doesn't solve such cases.
But killing old connection solves wait_timeout problem too.

Contributor

methane commented Sep 16, 2015

I'm sorry. I was wrong about wait_timeout.

But still there are some cases old connections should be killed.

  • setting global variable (e.g. set @@global.query_cache_type=0).
  • When adding new read-only slave to load balancer, or gracefully switching slave.

Killing not recently used connections doesn't solve such cases.
But killing old connection solves wait_timeout problem too.

@elgs

This comment has been minimized.

Show comment
Hide comment
@elgs

elgs Sep 17, 2015

I had the same problem when I was using JDBC - DBCP. DBCP allows me to specify a dummy query and the interval to run against the database. I set it to 1 hour so that there's no connection that could be idle for longer than 1 hour.

elgs commented Sep 17, 2015

I had the same problem when I was using JDBC - DBCP. DBCP allows me to specify a dummy query and the interval to run against the database. I set it to 1 hour so that there's no connection that could be idle for longer than 1 hour.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Nov 4, 2015

Contributor

@bradfitz, please review CL 6580 soon. Thanks.

Contributor

rsc commented Nov 4, 2015

@bradfitz, please review CL 6580 soon. Thanks.

@bradfitz bradfitz closed this in 0c516c1 Dec 2, 2015

@duyanghao

This comment has been minimized.

Show comment
Hide comment
@duyanghao

duyanghao Oct 18, 2016

@bradfitz Is there any time configuration for no-interaction?
what i mean is the max wait time when there is no interaction(Similar to the wait_timeout on the side of server).
for example:
the client socket will close itself after 30 seconds without any interaction with server.
Attention:
As far as i know,the SetConnMaxLifetime means the lifetime starting from created,but what i need is the lifetime starting from no-interaction.

duyanghao commented Oct 18, 2016

@bradfitz Is there any time configuration for no-interaction?
what i mean is the max wait time when there is no interaction(Similar to the wait_timeout on the side of server).
for example:
the client socket will close itself after 30 seconds without any interaction with server.
Attention:
As far as i know,the SetConnMaxLifetime means the lifetime starting from created,but what i need is the lifetime starting from no-interaction.

@methane

This comment has been minimized.

Show comment
Hide comment
@methane

methane Oct 18, 2016

Contributor

As far as i know,the SetConnMaxLifetime means the lifetime starting from created,but what i need is the lifetime starting from no-interaction.

"lifetime starting from no-interaction" < "lifetime starting from created"
So you can use SetConnMaxLifetime(time.Second*30) to avoid wait_timeout problem, when
wait_timeout=30.

SetConnMaxLifetime can be used to solve many problems. And I don't want to add more timeouts
and make database/sql more complicated.

Is there any reason you can't use SetConnMaxLifetime?

Contributor

methane commented Oct 18, 2016

As far as i know,the SetConnMaxLifetime means the lifetime starting from created,but what i need is the lifetime starting from no-interaction.

"lifetime starting from no-interaction" < "lifetime starting from created"
So you can use SetConnMaxLifetime(time.Second*30) to avoid wait_timeout problem, when
wait_timeout=30.

SetConnMaxLifetime can be used to solve many problems. And I don't want to add more timeouts
and make database/sql more complicated.

Is there any reason you can't use SetConnMaxLifetime?

@duyanghao

This comment has been minimized.

Show comment
Hide comment
@duyanghao

duyanghao Oct 18, 2016

@methane I don't think SetConnMaxLifetime work efficiently as what i mean timeout starting from no-interaction.
for example:
i want to let socket timeout(starting from no-interaction) after 30s,and what's more,i don't want the socket timeout(starting from created) after 30s, as i want to keep using the socket instead of killing it and recreating another socket to process http request when the program is busy processing requests!
after all,reusing the socket is more efficient than recreating another socket!

in other words,i want to keep socket lifetime(starting from created) as long as possible for efficient reuse of socket.And at the same time,i want to set socket lifetime(starting from no-interaction) only 30s for recycling the idle socket!

duyanghao commented Oct 18, 2016

@methane I don't think SetConnMaxLifetime work efficiently as what i mean timeout starting from no-interaction.
for example:
i want to let socket timeout(starting from no-interaction) after 30s,and what's more,i don't want the socket timeout(starting from created) after 30s, as i want to keep using the socket instead of killing it and recreating another socket to process http request when the program is busy processing requests!
after all,reusing the socket is more efficient than recreating another socket!

in other words,i want to keep socket lifetime(starting from created) as long as possible for efficient reuse of socket.And at the same time,i want to set socket lifetime(starting from no-interaction) only 30s for recycling the idle socket!

@methane

This comment has been minimized.

Show comment
Hide comment
@methane

methane Oct 18, 2016

Contributor

Before SetConnMaxLifetime, people tries to reduce idle connection via SetMaxIdleConns().
It caused dozen reconnection in each second, because "conns actually in use" is very
volatile.

Thanks to SetConnMaxLifetime, reconnection rate changed from "dozen /secs" to "few /minutes".
It was very huge improvement of efficiency. That's one of dozen problems SetMaxIdleConns() solved.

Now I'm not sure adding SetMaxIdleTimeout() improve efficiency dramatically.
But I'm sure it's add significant complexity to connection pool.

Is there good benchmark application which lack of SetMaxIdleTimeout cause major efficiency issue?

Contributor

methane commented Oct 18, 2016

Before SetConnMaxLifetime, people tries to reduce idle connection via SetMaxIdleConns().
It caused dozen reconnection in each second, because "conns actually in use" is very
volatile.

Thanks to SetConnMaxLifetime, reconnection rate changed from "dozen /secs" to "few /minutes".
It was very huge improvement of efficiency. That's one of dozen problems SetMaxIdleConns() solved.

Now I'm not sure adding SetMaxIdleTimeout() improve efficiency dramatically.
But I'm sure it's add significant complexity to connection pool.

Is there good benchmark application which lack of SetMaxIdleTimeout cause major efficiency issue?

@golang golang locked and limited conversation to collaborators Oct 18, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.