diff --git a/common/database/initialize.go b/common/database/initialize.go index 75c2bd68..60d6b6ba 100644 --- a/common/database/initialize.go +++ b/common/database/initialize.go @@ -41,6 +41,7 @@ func setupSimpleDatabase(host string, c *toolsConfig.Database) { if err != nil { log.Fatal(pkg.Red(err.Error())) } + open = withSqliteDefaults(c.Driver, open) resolverConfig := toolsDB.NewConfigure(c.Source, c.MaxIdleConns, c.MaxOpenConns, c.ConnMaxIdleTime, c.ConnMaxLifeTime, registers) db, err := resolverConfig.Init(&gorm.Config{ diff --git a/common/database/sqlite_dsn.go b/common/database/sqlite_dsn.go new file mode 100644 index 00000000..7da673ee --- /dev/null +++ b/common/database/sqlite_dsn.go @@ -0,0 +1,64 @@ +package database + +import ( + "net/url" + "strings" + + "gorm.io/gorm" +) + +// sqlitePragmas are the connection parameters a file-backed sqlite database +// needs to behave under a server that issues concurrent requests. Each one is +// applied only when the configured source does not already set it. +// +// Without them sqlite runs in rollback-journal mode with no busy timeout: a +// writer blocks every reader and a second writer fails at once with +// "database is locked", instead of waiting for the first to finish. +// +// _busy_timeout wait up to 10s for a lock rather than failing immediately +// _journal_mode WAL lets readers proceed while a write is in progress +// _synchronous NORMAL is the pairing WAL is designed for +// _txlock a transaction takes the write lock when it begins. The +// default defers it, and a deferred transaction that reads +// and then writes is refused with SQLITE_BUSY straight away - +// busy_timeout does not apply to that upgrade. +var sqlitePragmas = [][2]string{ + {"_busy_timeout", "10000"}, + {"_journal_mode", "WAL"}, + {"_synchronous", "NORMAL"}, + {"_txlock", "immediate"}, +} + +// sqliteDSN returns source with the defaults in sqlitePragmas added. +// In-memory databases are returned untouched: WAL has no meaning there, and +// every connection to one is its own database anyway. +func sqliteDSN(source string) string { + if source == "" || strings.Contains(source, ":memory:") || strings.Contains(source, "mode=memory") { + return source + } + + path, rawQuery := source, "" + if i := strings.IndexByte(source, '?'); i >= 0 { + path, rawQuery = source[:i], source[i+1:] + } + q, err := url.ParseQuery(rawQuery) + if err != nil { + // Not something to rewrite blind; the driver will report it. + return source + } + for _, p := range sqlitePragmas { + if _, set := q[p[0]]; !set { + q.Set(p[0], p[1]) + } + } + return path + "?" + q.Encode() +} + +// withSqliteDefaults wraps open so every connection string handed to a sqlite3 +// dialector - the primary and any replicas - carries the defaults above. +func withSqliteDefaults(driver string, open func(string) gorm.Dialector) func(string) gorm.Dialector { + if driver != "sqlite3" { + return open + } + return func(dsn string) gorm.Dialector { return open(sqliteDSN(dsn)) } +} diff --git a/common/database/sqlite_dsn_test.go b/common/database/sqlite_dsn_test.go new file mode 100644 index 00000000..e4da5ff4 --- /dev/null +++ b/common/database/sqlite_dsn_test.go @@ -0,0 +1,46 @@ +package database + +import ( + "net/url" + "testing" +) + +func TestSqliteDSN(t *testing.T) { + parse := func(s string) (string, url.Values) { + u, err := url.Parse(s) + if err != nil { + t.Fatalf("parse %q: %v", s, err) + } + return u.Path, u.Query() + } + + t.Run("plain path gets every default", func(t *testing.T) { + path, q := parse(sqliteDSN("./go-admin-db.db")) + if path != "./go-admin-db.db" { + t.Errorf("path = %q", path) + } + for _, p := range sqlitePragmas { + if q.Get(p[0]) != p[1] { + t.Errorf("%s = %q, want %q", p[0], q.Get(p[0]), p[1]) + } + } + }) + + t.Run("explicit values win", func(t *testing.T) { + _, q := parse(sqliteDSN("a.db?_busy_timeout=500&_journal_mode=DELETE")) + if q.Get("_busy_timeout") != "500" || q.Get("_journal_mode") != "DELETE" { + t.Errorf("overridden: %v", q) + } + if q.Get("_txlock") != "immediate" { + t.Errorf("missing default alongside overrides: %v", q) + } + }) + + t.Run("in-memory and empty untouched", func(t *testing.T) { + for _, s := range []string{"", ":memory:", "file::memory:?cache=shared", "file:x?mode=memory"} { + if got := sqliteDSN(s); got != s { + t.Errorf("sqliteDSN(%q) = %q", s, got) + } + } + }) +} diff --git a/common/database/sqlite_lock_test.go b/common/database/sqlite_lock_test.go new file mode 100644 index 00000000..aa5426d8 --- /dev/null +++ b/common/database/sqlite_lock_test.go @@ -0,0 +1,91 @@ +//go:build sqlite3 + +package database + +import ( + "path/filepath" + "sync" + "testing" + + "gorm.io/driver/sqlite" + "gorm.io/gorm" +) + +type lockRow struct { + ID int `gorm:"primaryKey"` + N int +} + +// hammer runs writers and readers against one file at the same time and +// returns how many statements failed. +func hammer(t *testing.T, dsn string) (failed int, firstErr error) { + t.Helper() + db, err := gorm.Open(sqlite.Open(dsn), &gorm.Config{}) + if err != nil { + t.Fatal(err) + } + if err := db.AutoMigrate(&lockRow{}); err != nil { + t.Fatal(err) + } + db.Create(&lockRow{ID: 1}) + + var mu sync.Mutex + note := func(err error) { + if err == nil { + return + } + mu.Lock() + failed++ + if firstErr == nil { + firstErr = err + } + mu.Unlock() + } + + var wg sync.WaitGroup + for w := 0; w < 8; w++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 40; i++ { + note(db.Create(&lockRow{N: i}).Error) + // read-then-write transaction: the shape a deferred + // transaction cannot upgrade without SQLITE_BUSY + note(db.Transaction(func(tx *gorm.DB) error { + var r lockRow + if err := tx.First(&r, 1).Error; err != nil { + return err + } + return tx.Model(&lockRow{}).Where("id = 1").Update("n", r.N+1).Error + })) + } + }() + } + for r := 0; r < 4; r++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 80; i++ { + var c int64 + note(db.Model(&lockRow{}).Count(&c).Error) + } + }() + } + wg.Wait() + return failed, firstErr +} + +func TestSqliteDefaultsPreventLocking(t *testing.T) { + file := filepath.Join(t.TempDir(), "lock.db") + if failed, err := hammer(t, sqliteDSN(file)); failed != 0 { + t.Fatalf("%d statements failed with the defaults applied, first: %v", failed, err) + } +} + +// Documents what the defaults are for: the bare path loses statements. +func TestSqliteBarePathLocks(t *testing.T) { + file := filepath.Join(t.TempDir(), "bare.db") + if failed, _ := hammer(t, file); failed == 0 { + t.Skip("bare path did not lock on this machine; the comparison proves nothing here") + } +}