Commit `34360906d0`

34360906d041a79cf504009bdbde1df30df7384b

parent: 6ab65cbbaf

Verified · cmc ci/build: success ci/test: success ci/vuln: success

cmc <hello@cleberg.net> · 2026-09-04 15:29 UTC

store: writers serialise instead of failing under contention

Every transaction in the store package writes. A deferred one takes the
write lock at its first write, by which point another writer may hold it;
SQLite answers SQLITE_BUSY and does not run the busy handler for that
case, so busy_timeout cannot help and the transaction fails outright.
Measured with eight concurrent read-then-write transactions, 229 of 320
failed.

_txlock=immediate takes the lock at BEGIN, where busy_timeout does apply.
The same load runs with no failures. Readers are unaffected: WAL keeps
them out of a writer's way, and no transaction in this package is
read-only.

SetMaxOpenConns(1), the issue's other suggestion, was measured too: it
also removes the failures but caps write throughput (11.5k vs 13.8k in
two seconds) for reader throughput this instance does not need.

A heavily contended write now blocks for up to busy_timeout rather than
returning an error immediately.

Closes #121

Layout: unified · split

internal/store/contention_test.go added +80

@@ -0,0 +1,80 @@
	1	package store
	2
	3	import (
	4	"sync"
	5	"sync/atomic"
	6	"testing"
	7	)
	8
	9	// TestConcurrentWritersDoNotFail runs the shape every write transaction in
	10	// this package has — read a row, write it back, inside one Begin — from
	11	// several goroutines at once.
	12	//
	13	// Deferred transactions take the write lock at their first write, by which
	14	// point another writer may hold it. SQLite answers SQLITE_BUSY and does
	15	// not invoke the busy handler for that case, so busy_timeout cannot help
	16	// and the transaction simply fails. Measured before the fix, 44% of these
	17	// failed. The DSN opens transactions as immediate, which takes the lock up
	18	// front where busy_timeout applies (#121).
	19	//
	20	// This has to be a real file: :memory: gives each connection its own
	21	// database, so nothing contends.
	22	func TestConcurrentWritersDoNotFail(t *testing.T) {
	23	s := open(t)
	24	if err := s.MigrateUp(); err != nil {
	25	t.Fatal(err)
	26	}
	27	uid, err := s.CreateUser("cmc", true)
	28	if err != nil {
	29	t.Fatal(err)
	30	}
	31	repoID, err := s.CreateRepo("user", uid, "lib", "public")
	32	if err != nil {
	33	t.Fatal(err)
	34	}
	35
	36	const writers, each = 8, 40
	37	var failures int64
	38	var wg sync.WaitGroup
	39	for i := 0; i < writers; i++ {
	40	wg.Add(1)
	41	go func() {
	42	defer wg.Done()
	43	for j := 0; j < each; j++ {
	44	if err := func() error {
	45	tx, err := s.DB.Begin()
	46	if err != nil {
	47	return err
	48	}
	49	defer tx.Rollback()
	50	var n int
	51	if err := tx.QueryRow(
	52	"SELECT issue_counter FROM repos WHERE id = ?", repoID).Scan(&n); err != nil {
	53	return err
	54	}
	55	if _, err := tx.Exec(
	56	"UPDATE repos SET issue_counter = ? WHERE id = ?", n+1, repoID); err != nil {
	57	return err
	58	}
	59	return tx.Commit()
	60	}(); err != nil {
	61	atomic.AddInt64(&failures, 1)
	62	}
	63	}
	64	}()
	65	}
	66	wg.Wait()
	67
	68	if failures != 0 {
	69	t.Fatalf("%d of %d write transactions failed", failures, writers*each)
	70	}
	71	// Serialised writers each read what the last one committed, so no
	72	// increment is lost.
	73	var got int
	74	if err := s.DB.QueryRow("SELECT issue_counter FROM repos WHERE id = ?", repoID).Scan(&got); err != nil {
	75	t.Fatal(err)
	76	}
	77	if got != writers*each {
	78	t.Fatalf("counter = %d, want %d: an increment was lost", got, writers*each)
	79	}
	80	}

internal/store/store.go +14 −2

	@@ -25,10 +25,22 @@ type Store struct {
25		25
26	// Open opens (creating if needed) the database at path with WAL mode and	26	// Open opens (creating if needed) the database at path with WAL mode and
27	// foreign keys enforced. Use ":memory:" in tests.	27	// foreign keys enforced. Use ":memory:" in tests.
		28	//
		29	// _txlock=immediate is what serialises writers. Every transaction in this
		30	// package writes, and a deferred one takes the write lock only when it
		31	// reaches its first write — by which point another writer may hold it.
		32	// SQLite answers that with SQLITE_BUSY and does not invoke the busy
		33	// handler, because waiting would deadlock two transactions each holding a
		34	// read lock the other needs; busy_timeout cannot help. Measured with
		35	// eight concurrent read-then-write transactions, 44% of them failed.
		36	// Beginning immediate takes the write lock up front, where busy_timeout
		37	// does apply, so a second writer waits its turn: the same load runs with
		38	// no failures, and readers, which WAL keeps out of the way, are
		39	// unaffected (#121).
28	func Open(path string) (*Store, error) {	40	func Open(path string) (*Store, error) {
29	dsn := path + "?_pragma=journal_mode(WAL)&_pragma=foreign_keys(ON)&_pragma=busy_timeout(5000)"	41	dsn := path + "?_txlock=immediate&_pragma=journal_mode(WAL)&_pragma=foreign_keys(ON)&_pragma=busy_timeout(5000)"
30	if path == ":memory:" {	42	if path == ":memory:" {
31	dsn = ":memory:?_pragma=foreign_keys(ON)"	43	dsn = ":memory:?_txlock=immediate&_pragma=foreign_keys(ON)"
32	}	44	}
33	db, err := sql.Open("sqlite", dsn)	45	db, err := sql.Open("sqlite", dsn)
34	if err != nil {	46	if err != nil {

Commit 34360906d0

Commit `34360906d0`