From ea679bf00bd3a52c63e7296fc121c7f9b1bacf10 Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Wed, 7 Oct 2026 17:32:39 +0800 Subject: [PATCH] =?UTF-8?q?fix=F0=9F=90=9B:=20give=20sqlite=20connections?= =?UTF-8?q?=20a=20busy=20timeout,=20WAL=20and=20immediate=20transactions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 1 + common/database/sqlite_dsn.go | 64 ++++++++++++++++++++ common/database/sqlite_dsn_test.go | 46 +++++++++++++++ common/database/sqlite_lock_test.go | 91 +++++++++++++++++++++++++++++ 4 files changed, 202 insertions(+) create mode 100644 common/database/sqlite_dsn.go create mode 100644 common/database/sqlite_dsn_test.go create mode 100644 common/database/sqlite_lock_test.go 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") + } +}