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: SetConnMaxLifetime may be ignored #27793

Closed
AlekSi opened this issue Sep 21, 2018 · 3 comments
Closed

database/sql: SetConnMaxLifetime may be ignored #27793

AlekSi opened this issue Sep 21, 2018 · 3 comments

Comments

@AlekSi
Copy link
Contributor

@AlekSi AlekSi commented Sep 21, 2018

https://golang.org/pkg/database/sql/#DB.SetConnMaxLifetime says:

SetConnMaxLifetime sets the maximum amount of time a connection may be reused.
Expired connections may be closed lazily before reuse.
If d <= 0, connections are reused forever.

After reading that one can expect that db.SetConnMaxLifetime(time.Nanosecond) will prevent connection reuse. Yes, there is a "lazy" in the second sentence, but the first one is straightforward.

But SetConnMaxLifetime silently rises to duration to time.Second:

go/src/database/sql/sql.go

Lines 890 to 896 in ce58a39

func (db *DB) connectionCleaner(d time.Duration) {
const minInterval = time.Second
if d < minInterval {
d = minInterval
}
t := time.NewTimer(d)

I propose to change the documentation to use a weaker language.

@kardianos
Copy link
Contributor

@kardianos kardianos commented Sep 22, 2018

A nanosecond is great then 0, so you get the lazy verbage. Maybe I'm wrong. I'll look at it more.

@kardianos
Copy link
Contributor

@kardianos kardianos commented Sep 30, 2018

@AlekSi I'm reading the documentation again, and I'm not sure I see your point of confusion. Do you have a suggestion for the language that would be clearer to you? If anything I would make the language stronger to state the 1s minimum timer.

@kardianos
Copy link
Contributor

@kardianos kardianos commented Oct 27, 2018

I think the documentation as it stands is correct. Also I think that the exact method we use may change in the future, so I'm happy with how it is.

@kardianos kardianos closed this Oct 27, 2018
@golang golang locked and limited conversation to collaborators Oct 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.