diff --git a/internal/store/store.go b/internal/store/store.go index 7ef6e78..1f42b19 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -38,6 +38,20 @@ func Open(path string) (*DB, error) { "_pragma=busy_timeout(5000)", "_pragma=foreign_keys(ON)", "_pragma=synchronous(NORMAL)", + // Every write transaction takes the write lock up front. + // + // Without this, `BEGIN` is deferred: a transaction starts as a reader and + // asks for the write lock at its first UPDATE. SQLite refuses that upgrade + // with SQLITE_BUSY *immediately* and does not apply busy_timeout to it — + // backing off cannot help, because the transaction is already holding a + // read snapshot that the other writer is invalidating. So the timeout above + // looks like it covers writer contention and covers none of it: two people + // saving at the same moment get a 500 in two milliseconds. + // + // `immediate` takes the lock at BEGIN instead, where busy_timeout does + // apply and the second writer waits its turn. The driver leaves read-only + // transactions deferred, so readers still run concurrently. + "_txlock=immediate", // Keep the temp b-trees FTS5 and ORDER BY build in memory rather than // spilling them into the data volume. "_pragma=temp_store(MEMORY)", diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 29450df..0013413 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -2,6 +2,7 @@ package store import ( "context" + "fmt" "path/filepath" "testing" ) @@ -95,6 +96,51 @@ func TestPragmasAreSet(t *testing.T) { } } +// Two people saving at the same moment must both succeed. +// +// This is a regression test for a specific and badly-behaved failure: with the +// default deferred BEGIN, a write transaction starts as a reader and asks for the +// write lock partway through. SQLite refuses that upgrade with SQLITE_BUSY at once +// and does *not* apply busy_timeout to it — so the contention surfaced as a 500 +// two milliseconds after the request arrived, while every single-threaded test +// passed. `_txlock=immediate` is what makes the timeout actually cover writers. +func TestConcurrentWritersDoNotHitSQLiteBusy(t *testing.T) { + db := openTest(t) + ctx := context.Background() + seedTaskFixtures(t, db) + + const writers = 8 + errs := make(chan error, writers) + + start := make(chan struct{}) + for i := range writers { + go func(i int) { + <-start // all of them ask for the write lock at once + errs <- db.CreateTask(ctx, &Task{ + ProjectID: "p1", + Content: fmt.Sprintf("opgave %d", i), + CreatedBy: "u1", + }, nil) + }(i) + } + close(start) + + for range writers { + if err := <-errs; err != nil { + t.Fatalf("concurrent write failed: %v", err) + } + } + + var n int + if err := db.QueryRowContext(ctx, + `SELECT count(*) FROM tasks WHERE project_id = 'p1'`).Scan(&n); err != nil { + t.Fatal(err) + } + if n != writers { + t.Errorf("wrote %d tasks, want %d", n, writers) + } +} + // The FTS index is kept in step with `tasks` by triggers rather than by application // code, so a task written by any path at all is searchable. That contract is worth // a test of its own.