Using driver/sql hangs database and other oddities #210
Description
I can reproduce mattn/go-sqlite3#204 with minimal changes.
main1.go
// https://github.com/mattn/go-sqlite3/issues/705
package main
import (
"database/sql"
_ "github.com/genjidb/genji/sql/driver"
)
func main() {
db, err := sql.Open("genji", ":memory:")
if err != nil {
panic(err)
}
_, err = db.Exec("CREATE TABLE m")
if err != nil {
panic(err)
}
_, err = db.Query("SELECT * FROM m") // note that I didn’t close the rows
if err != nil {
panic(err)
}
_, err = db.Query("SELECT * FROM m")
if err != nil {
panic(err) // table not found
}
panic("unreachable")
}
main2.go
package main
import (
"context"
"database/sql"
_ "github.com/genjidb/genji/sql/driver"
)
func main() {
ctx := context.Background()
db, err := sql.Open("genji", "test.db")
if err != nil {
panic(err)
}
db.Conn(ctx)
db.Conn(ctx)
panic("unreachable")
}
I assume SQLite releases db locks when they are not needed. But this issue gets worse with Genji since BoltDB blocks on db lock (unless a timeout was specified, which is not currently possible with sql/driver), and BadgerDB I believe just returns an error. That is, things go terribly wrong when sql.DB attempts to open a new sql.Conn.
As a temporary workaround it’s possible to call SetMaxOpenConns(1) on *sql.DB from the user’s code. This should work just fine for in-memory and bolt engines, but afaik badger supports concurrent transactions whereas single driver.Conn can’t be used concurrently.
I’ve experimented a bit with possible APIs to fix those issues, and I think the best option would be to fix this use case on the Go side by allowing driver.Connector to implement optional io.Closer interface and close it in sql.DB.Close. Then driver.Connector would be able to hold genji.DB object and release the resources when sql.DB is no longer needed.
Alternatively, it’s possible to do reference counting on genji.DB in driver.Connector and driver.Conn, and have a special case for in-memory engines. That has a benefit of releasing the database handle when idle (e.g. so that other processes may perform some maintenance tasks).
- Also, it looks like
driver.Connis reusing itself asdriver.Txfor transactions (lines 117–123). This won’t work as expected if we begin multiple transactions on a single connection.
https://github.com/genjidb/genji/blob/112fd24e38b07590fd077be043837e0425731a8a/sql/driver/driver.go#L108-L124