Skip to content
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: add lastUseTime or similar to driverConn, add SetConnMaxIdleLefttime to DB #25232

Closed
leaxoy opened this issue May 3, 2018 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Milestone

Comments

@leaxoy
Copy link

leaxoy commented May 3, 2018

I noticed some db like mysql support idle timeout, which means a conn will closed when timeout. In sql package, only has db.SetConnMaxLefttime and driverConn.createdAt available, so I suggest add lastUseTime to driverConn and SetConnMaxIdleLefttime to DB.

@leaxoy leaxoy changed the title [feature] database/sql add lastUseTime or similar to driverConn, add SetConnMaxIdleLefttime to DB proposal: database/sql: add lastUseTime or similar to driverConn, add SetConnMaxIdleLefttime to DB May 3, 2018
@gopherbot gopherbot added this to the Proposal milestone May 3, 2018
@kardianos
Copy link
Contributor

I don't fully follow what you are suggesting. However, if a driver connection sets an idle timeout in the DSN or settings then the connection will close on idle. If that connection (still in the pool) attempts to be used, it will return driver.ErrBadConn and the pool will try to pull another one.

Actually, it may not be a bad idea to support a method to signal to the database connection pool that the driver is bad right away rather then retrying and risk getting all bad connections. I'm not sure how that would work of hand.

@leaxoy
Copy link
Author

leaxoy commented May 5, 2018

Thanks for your prompt, I find an another way to keep conns alive by Ping it by time ticker.

@leaxoy
Copy link
Author

leaxoy commented May 5, 2018

Sadly, I also had no idea how to achieve this feature now. But we can talk it in the future

@kardianos
Copy link
Contributor

I think you are proposing a new database pool setting called "set max idle time" where if the connection is idle for ever the max time spent it is closed and removed from the pool.

Is this correct?

@leaxoy
Copy link
Author

leaxoy commented May 8, 2018

Yeah, thanks for your detailed explanation

@rsc
Copy link
Contributor

rsc commented Jul 23, 2018

@kardianos, does this proposal sound good to you? OK to accept if so.

@gopherbot
Copy link

Change https://golang.org/cl/145758 mentions this issue: database/sql: add SetConnMaxIdleTime

@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 8, 2019
@bradfitz bradfitz changed the title proposal: database/sql: add lastUseTime or similar to driverConn, add SetConnMaxIdleLefttime to DB database/sql: add lastUseTime or similar to driverConn, add SetConnMaxIdleLefttime to DB Feb 8, 2019
@bradfitz bradfitz modified the milestones: Proposal, Go1.13 Feb 8, 2019
@methane
Copy link
Contributor

methane commented Apr 3, 2019

@leaxoy Why SetMaxLifetime() is not enough?

When idle_timeout is 3600sec, SetMaxLifetime(time.Second * 1800) can avoid idle timeout.

I don't want to add MaxIdleTime, because it makes pool more complicated.

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@smasher164 smasher164 modified the milestones: Backlog, Go1.14 Oct 11, 2019
@odeke-em
Copy link
Member

Thank you for sending CL 145758 @kardianos, but unfortunately we didn't get it in for Go1.14. @methane would you mind commenting on the tagged CL about how this new method won't work for you, or how you can use the previous available methods?

Thank you.

@odeke-em odeke-em modified the milestones: Go1.14, Backlog Nov 29, 2019
@golang golang locked and limited conversation to collaborators Feb 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Projects
None yet
Development

No branches or pull requests

9 participants