fix🐛: give sqlite connections a busy timeout, WAL and immediate transactions

A file-backed sqlite database ran in rollback-journal mode with no busy
timeout, so concurrent requests failed with "database is locked" instead of
waiting for each other. Add the defaults to the sqlite3 connection string
unless the configured source already sets them.

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{

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)) }
+}

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)
+			}
+		}
+	})
+}

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")
+	}
+}