fix(store): bump SQLite busy_timeout from 5s to 30s (BUG-853) (#277)

TestSQLiteConcurrentWritersNoBusy intermittently fails on the GitHub-
hosted Go (SQLite) CI job with `database is locked (5) (SQLITE_BUSY)`
under 25 concurrent writers × 5 ops. The test asserts ZERO errors so
that BUG-748's `_txlock=immediate` regression stays pinned — but with
the DSN's busy_timeout at 5s, the unluckiest writer on a slow shared
runner can exceed the timeout: 125 serialized inserts under heavy
contention from sibling test packages add up.

Bumping busy_timeout to 30s gives a 6× margin over the worst observed
CI run and ~50× the normal local p95. Genuine deadlocks don't happen
with WAL + BEGIN IMMEDIATE, so the only thing the higher value costs
is "how long we wait before declaring lock contention pathological".
For Pad's workload, 30s is fine.

Surfaced once BUG-851 (PR #276) cleared the rate-limiter goroutine
leak that had been masking everything else on main.

Verified locally:

    $ go test -count=20 -run TestSQLiteConcurrentWritersNoBusy \
        ./internal/store/
    ok  github.com/PerpetualSoftware/pad/internal/store  6.102s

Note: this is a single-character DSN change plus a doc-comment update;
no code path or contract is altered. Read concurrency (WAL) is
unchanged — we're not touching SetMaxOpenConns.
This commit is contained in:
xarmian
2026-04-28 17:59:21 -04:00
committed by GitHub
parent 715ec70e94
commit 53b5add4e9
+17 -5
View File
@@ -38,10 +38,22 @@ func (s *Store) DB() *sql.DB { return s.db }
//
// The DSN is configured for safe concurrent use under Go's connection pool:
//
// - `_pragma=busy_timeout(5000)`: when a connection finds the database
// locked, retry for up to 5 seconds before returning SQLITE_BUSY. The
// pragma is applied per-connection by the driver, so every pool member
// inherits it.
// - `_pragma=busy_timeout(30000)`: when a connection finds the database
// locked, retry for up to 30 seconds before returning SQLITE_BUSY.
// The pragma is applied per-connection by the driver, so every pool
// member inherits it.
//
// 30s is much larger than the millisecond-scale p95 we observe under
// normal load — the slack is there to absorb shared-runner jitter
// under heavy CI contention. With the previous 5s value,
// TestSQLiteConcurrentWritersNoBusy (25 writers × 5 ops, all
// contending for BEGIN IMMEDIATE) intermittently failed on the
// GitHub-hosted SQLite job: the cumulative wall-time of 125
// serialized inserts on a slow runner can exceed 5s, leaving the
// unluckiest writer to time out. See BUG-853. Genuine deadlocks
// don't happen with WAL + BEGIN IMMEDIATE, so the only thing the
// larger value costs is "how long we wait before declaring lock
// contention pathological"; for Pad's workload, 30s is fine.
//
// - `_pragma=foreign_keys(on)`: foreign-key enforcement is per-connection
// in SQLite. Setting it via the DSN guarantees ALL pool members enforce
@@ -87,7 +99,7 @@ func (s *Store) DB() *sql.DB { return s.db }
// setting (stored in the file header), so it persists across
// connections after the first one applies it.
func New(dbPath string) (*Store, error) {
dsn := dbPath + "?_pragma=busy_timeout(5000)&_pragma=foreign_keys(on)&_txlock=immediate"
dsn := dbPath + "?_pragma=busy_timeout(30000)&_pragma=foreign_keys(on)&_txlock=immediate"
db, err := sql.Open("sqlite", dsn)
if err != nil {
return nil, fmt.Errorf("open database: %w", err)