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: confusing MaxIdleClosed statistic #27792

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

database/sql: confusing MaxIdleClosed statistic #27792

AlekSi opened this issue Sep 21, 2018 · 10 comments
Assignees
Milestone

Comments

@AlekSi
Copy link
Contributor

@AlekSi AlekSi commented Sep 21, 2018

What version of Go are you using (go version)?

Both Go 1.11 and the current tip. MaxIdleClosed is not available in Go 1.10.

What did you do?

// use single database connection, never close it, always reuse
db.SetMaxOpenConns(1)
db.SetMaxIdleConns(1)
db.SetConnMaxLifetime(0)

log.Printf("%+v", db.Stats())

for i := 0; i < 10; i++ {
	if _, err := db.Exec("SELECT 1"); err != nil {
		log.Fatal(err)
	}
}

log.Printf("%+v", db.Stats())

https://github.com/AlekSi/go-sql-bugs/blob/master/issue27792_test.go

What did you expect to see?

{MaxOpenConnections:1 OpenConnections:0 InUse:0 Idle:0 WaitCount:0 WaitDuration:0s MaxIdleClosed:0 MaxLifetimeClosed:0}
{MaxOpenConnections:1 OpenConnections:1 InUse:0 Idle:1 WaitCount:0 WaitDuration:0s MaxIdleClosed:0 MaxLifetimeClosed:0}

What did you see instead?

{MaxOpenConnections:1 OpenConnections:0 InUse:0 Idle:0 WaitCount:0 WaitDuration:0s MaxIdleClosed:0 MaxLifetimeClosed:0}
{MaxOpenConnections:1 OpenConnections:1 InUse:0 Idle:1 WaitCount:0 WaitDuration:0s MaxIdleClosed:10 MaxLifetimeClosed:0}

By patching a driver I noticed that only a single connection is established in fact. But the statistic is wrong.

/cc @kardianos

@AlekSi
Copy link
Contributor Author

@AlekSi AlekSi commented Sep 21, 2018

Both Go 1.11 and the current tip. MaxIdleClosed is not available in Go 1.10.

btw, it is not mentioned on https://golang.org/doc/go1.11 at all. @dmitshur do you update changelogs after the fact? Should I create a separate issue?

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Sep 21, 2018

btw, it is not mentioned on https://golang.org/doc/go1.11 at all. @dmitshur do you update changelogs after the fact? Should I create a separate issue?

The release notes only cover the most noteworthy changes or additions; it's not meant to be an exhaustive list of minor changes to the standard library. The package documentation is the place to find detailed information.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 22, 2018

@bcmills bcmills added this to the Go1.12 milestone Sep 22, 2018
@kardianos
Copy link
Contributor

@kardianos kardianos commented Sep 22, 2018

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 22, 2018

Great, thank you!

@kardianos
Copy link
Contributor

@kardianos kardianos commented Sep 23, 2018

@AlekSi I see how this happens. I don't have a solution yet. I'll continue working on it.

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 30, 2018

Change https://golang.org/cl/138578 mentions this issue: database/sql: correctly report MaxIdleClosed stat

@gopherbot gopherbot closed this in 7db509e Oct 2, 2018
@jameshartig
Copy link
Contributor

@jameshartig jameshartig commented Oct 22, 2018

Could get cherry-picked into a 1.11.x release?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 22, 2018

@gopherbot please open backport to 1.11

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 22, 2018

Backport issue(s) opened: #28325 (for 1.11).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

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
7 participants
You can’t perform that action at this time.