database/sql: close driver.Connector if it implements an io.Closer

[tie]

I propose adding a new sql.DB.Close behavior/side effect: closing driver.Connector if it implements an optional io.Closer interface.

See CL 258360 / #41710

We would like to be able to manage the resources in driver.Connector, e.g. to share the same underlying database handle between multiple connections. This is required for some embedded databases with in-memory backends like SQLite and Genji so that we don’t create a new in-memory store whenever sql.DB decides to ask for a new connection by calling driver.Connector.Connect.

For in-memory storage go-sqlite3 driver docs currently advice manually setting cache=shared DSN parameter, and non-zero max idle connection limit and infinite connection lifetime on sql.DB. (see FAQ: Why I'm getting no such table error?).

It get worse with Genji since both on-disk BoltDB and BadgerDB engines lock the database. That is, connector can’t open the database until the existing handle is closed. With database/sql it means that either user must set connection limit to 1, or we have to share the handle between driver.Conn instances, which also allows us to support concurrent transactions with BadgerDB (see genjidb/genji#210).

That said, we need to release the resources held by driver.Connector (e.g. close file handle, unlock database or free unmanaged memory) when sql.DB is no longer needed.

[ianlancetaylor]

CC @kardianos

[kardianos]

The bar to expand or update the existing database/sql (v1) is fairly high

However, the change is simple and won't interact with anything else. There is one other situation that this would help. If a driver for a database without inline connection cancelation has a background goroutine/connection dedicated to closing connections, this would provide a way to prevent that connection from leaking.

I would say tentatively accept.

[rsc]

@kardianos, we'll take care of updating the proposal project status. It needs to go through some discussion before moving to likely accept. Thanks.

[rsc]

Adding to minutes.

[rsc]

Does anyone object to adding this?

[rsc]

Based on the discussion, this seems like a likely accept.

[rsc]

No change in consensus, so accepted.

[gopherbot]

Change https://golang.org/cl/258360 mentions this issue: database/sql: close driver.Connector if it implements an io.Closer