mattn/go-sqlite3

"table not found" errors when concurrently reading from an in memory db

jmunson opened this issue ยท 17 comments

I ran into a bug where occasionally I was getting "table not found" errors for a database we create on startup and never write to or modify. I managed to reproduce it in the following gist

https://gist.github.com/jmunson/7b1974215d34439688b0 (name it main_test.go in an empty dir and go test -v should work)

This test file will create an in memory db and create a key->value table with 3 entries, then we spawn 20 goroutines that each query for all 3 entries.

If this is ran with GOMAXPROCS set to 2 or more without setting MaxOpenConns to 1, then we get "table not found" errors.

I'm not sure if this is something that can be fixed or should just be documented around, but I thought I'd share in case anyone else runs into this.

I've gotten this too. The reason is that each connection created by database/sql opens ":memory:", and thus gets a different in-memory database. Call sql.Open("sqlite3", "file::memory:?cache=shared") and this won't happen.

Just run into this. As a possible fix, go-sqlite3 could warn if ":memory:" is used as the DB path and the cache parameter is not explicitly provided as either 'shared' or 'private'. Thoughts?

I think a note in the Go docs that this is a common mistake, deferring to the SQLite documentation on the matter is better. When wrap something and introduce new behaviours in the wrapper, you only create further confusion.

I think that's a fair point, except that it may not be obvious to newcomers that database/sql is using a pool of connections and how this interacts with :memory:

After doing some further performance work on my current app, it actually looks like connection pooling results a pretty severe performance penalty with SQLite and that a single open connection is a better choice in any case. I've yet to write a proper benchmark for that and understand why that is though.

Was there any resolution to what the best method of fixing this was?

mattn commented

If you use :memory:, connection will be separated in the goroutine.

@Brian-McM based on what @robertknight said, I used https://golang.org/pkg/database/sql/#DB.SetMaxOpenConns to set the DB.SetMaxOpenConns to 1.

DB.SetMaxOpenConns(1) // this resolved the "table not found" in memory error for me.

I think I ended up doing something that, but it was pretty long time ago @domthinks so hard to remember.

mattn commented

did you try file=xxx.db?mode=memory ?

Just ran into this file::memory:?cache=shared and file:thisisauniquenamebutnotafile?mode=memory&cache=shared worked for me. This should be documented. Maybe adapt the documentation for the Open method to give more examples and add a warning right there? Does that sound OK?

I do not thing that ".SetMaxOpenConns(1)" should be used since then the advantages of having more than one connection are simply gone. Also, it might be possible that a connection is dropped/expires and then SQLite just drops the database. So this can happen with one connection too. Maybe it would be wise to have the shared argument for mode=memory per default?

mattn commented

This is related on the spec or behavior on sqlite3. If we will add this into doc, it will be FAQ style. However I'm not native speaker. Anyone, could you please send me pull-request to add this FAQ?

tych0 commented

Hi guys,

I just hit this too, and sent a PR with a FAQ entry. Here's a minimal reproducer that I wrote to prove to myself this would work:

package main

import (
	"database/sql"
	"fmt"
	"sync"

	"github.com/mattn/go-sqlite3"
)

type inserter interface {
	Exec(string, ...interface{}) (sql.Result, error)
}

func insert(i inserter, s string) error {
	for {
		_, err := i.Exec("INSERT INTO foo (name) VALUES (?)", s)
		if err == nil {
			return nil
		}
		if err == sqlite3.ErrLocked || err == sqlite3.ErrBusy || err.Error() == "database table is locked" {
			continue
		}
		return err
	}
}

func main() {
	letters := "abcdefghijklmnopqrstuvwxyz"

	//db, err := sql.Open("sqlite3", "file::memory:?mode=memory&cache=shared")
	db, err := sql.Open("sqlite3", ":memory:")
	if err != nil {
		panic(err)
	}

	_, err = db.Exec("CREATE TABLE foo (id INTEGER NOT NULL PRIMARY KEY, name TEXT)", nil)
	if err != nil {
		panic(err)
	}

	err = insert(db, "hello")
	if err != nil {
		panic(err)
	}

	wg := sync.WaitGroup{}
	wg.Add(len(letters))

	for _, l := range letters {
		go func(letter string) {
			defer wg.Done()

			tx, err := db.Begin()
			if err != nil {
				panic(err)
			}

			err = insert(tx, letter)
			if err != nil {
				panic(err)
			}

			err = tx.Commit()
			if err != nil {
				panic(err)
			}
		}(string(l))
	}

	err = insert(db, "goodbye")
	if err != nil {
		panic(err)
	}

	wg.Wait()

	rows, err := db.Query("SELECT name FROM foo")
	if err != nil {
		panic(err)
	}

	for rows.Next() {
		n := ""
		rows.Scan(&n)

		fmt.Printf("%v\n", n)
	}
}

Just for future reference, if you want to use a new in-memory db, e.g. for each new test case you can create a unique random string and place it in the filename:

x := randomString(16) // func creates random string
db, err := sql.Open("sqlite3", fmt.Sprintf("file:%s?mode=memory&cache=shared", x))

Do this simply for each TestFunc and the DBs will be separated.

Here is what worked for me.
package main

import (
"database/sql"
"fmt"
_ "github.com/mattn/go-sqlite3" // adding _ underscore due connection  error
"html/template"
"net/http"
   )

   type Page struct {
 Name     string
 DBStatus bool
 }

func main() {
  templates := template.Must(template.ParseFiles("template/index.html"))
    //Connect to database
  db, err := sql.Open("sqlite3", "./dev.db")
 checkErr(err)

http.HandleFunc("/", func(rw http.ResponseWriter, req *http.Request) {
	p := Page{Name: "John"}
	if name := req.FormValue("name"); name != "" {
		p.Name = name
	}
	db.SetMaxOpenConns(1) // added this due to connection error
	p.DBStatus = db.Ping() == nil
	if err := templates.ExecuteTemplate(rw, "index.html", p); err != nil {
		http.Error(rw, err.Error(), http.StatusInternalServerError)
	}
})

   fmt.Println(http.ListenAndServe(":8000", nil))
 }

  func checkErr(err error) {
if err != nil {
	panic(err)
 }
 }

One of the reasons using db.SetMaxOpenConns(1) might be bad is that trying to execute a query while having an open *sql.Rows will block because the object holds the only connection.

@alkemir Do you mean something like this?

rows, err := db.Query("SELECT FooID FROM Foo")
if err != nil {
    return err
}
for rows.Next() {
    var fooID int
    if err := rows.Scan(&fooID); err != nil {
        return err
    }
    if _, err := db.Exec("INSERT INTO Bar (FooID) VALUES (?001)", fooID); err != nil {
        return err
    }
}
return rows.Err()

If so, the problem is resolved by using a transaction, or the relatively new DB.Conn method. Both of these will allow the queries to run even though there's an open *sql.Rows object.

tx, err := db.Begin()
if err != nil {
    return err
}
defer tx.Rollback()

rows, err := tx.Query("SELECT FooID FROM Foo")
if err != nil {
    return err
}
for rows.Next() {
    var fooID int
    if err := rows.Scan(&fooID); err != nil {
        return err
    }
    if _, err := tx.Exec("INSERT INTO Bar (FooID) VALUES (?001)", fooID); err != nil {
        return err
    }
}

if err := rows.Err(); err != nil {
    return err
}

return tx.Commit()

Thats exactly what I mean. Thank you for posting alternatives! I think it is a pity that
application semantics have to accommodate for performance tuning. I feel there is
supposed to be a separation of concerns thats violated here. But of course, this is
more related to how the sql/database package relates with sqlite than this particular
repo.