From 792ca946f8de26bdcb3ca83ed02353be49bb9997 Mon Sep 17 00:00:00 2001 From: Sandro Date: Thu, 29 Oct 2026 14:51:01 +0100 Subject: [PATCH] fix(db): transactions wait for the write lock instead of failing (FLAW-23) A load test with 16 concurrent writers lost 12 of 480 writes with SQLITE_BUSY: transactions that read before writing cannot be helped by busy_timeout once another connection writes. _txlock=immediate makes every transaction take the write lock at BEGIN, so it queues instead. Co-Authored-By: Claude Opus 5.5 --- Doku/flaws.md | 3 +- internal/storage/concurrency_test.go | 62 ++++++++++++++++++++++++++++ internal/storage/sqlite.go | 5 ++- 3 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 internal/storage/concurrency_test.go diff --git a/Doku/flaws.md b/Doku/flaws.md index 741d644..9e09788 100644 --- a/Doku/flaws.md +++ b/Doku/flaws.md @@ -32,7 +32,7 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** ( | FLAW-20 | Qualität | ~~Sehr große Dateien~~; duplizierte Zugriffslogik | Niedrig | | FLAW-21 | Repo | ~~`LICENSE` fehlt, obwohl README MIT angibt~~ | **Behoben** | | FLAW-22 | Netzwerk | ~~Token in der URL (WS/SSE), CORS `*`, `CheckOrigin` immer `true`~~ | **Behoben** | -| FLAW-23 | SQLite | Single-Writer-Engpass bei vielen gleichzeitigen Schreibern | Niedrig | +| FLAW-23 | SQLite | ~~`SQLITE_BUSY` bei vielen gleichzeitigen Schreibern~~ | **Behoben** | | FLAW-24 | Gruppen | ~~Organisations-Endpunkte ohne Berechtigungsprüfung~~ | **Behoben** | | FLAW-25 | Workspace | ~~Struktur-Aktionen an fremd gesperrten Bullets nur teilweise ausgeführt~~ | **Behoben** | | FLAW-26 | Workspace | ~~Nach Enter landet sehr schnell Getipptes noch im vorherigen Bullet~~ | **Behoben** | @@ -181,6 +181,7 @@ Schweregrade: **Hoch** (Datenabfluss oder Rechteumgehung möglich), **Mittel** ( - **Maßnahme:** Kurzlebiges, einmaliges Stream-Ticket statt Langzeit-JWT in der URL; Origin gegen `BASE_URL` prüfen. ### FLAW-23: SQLite-Schreibengpass +- **Status: Behoben.** Transaktionen starten mit der Schreibsperre (`_txlock=immediate` im DSN). Vorher scheiterte eine Transaktion, die erst liest und dann schreibt, sofort mit `SQLITE_BUSY`, sobald jemand anderes schrieb (`busy_timeout` greift in diesem Fall nicht); im Lasttest 12 von 480 Schreibvorgängen. Jetzt warten Schreiber bis zu 10 s auf ihren Platz. Es bleibt ein Schreiber zur Zeit, für Familien und Teams ausreichend. Test: `TestConcurrentWritersDoNotFail`. - Eine SQLite-Datei (WAL, `busy_timeout`, bis zu 8 offene Verbindungen) erlaubt nur einen gleichzeitigen Schreiber. Für Familien und kleine Teams unkritisch; bei vielen aktiven Editoren sind `SQLITE_BUSY`-Fehler möglich, die wegen FLAW-14 stumm bleiben. ### FLAW-24: Organisations-Endpunkte ohne Berechtigungsprüfung diff --git a/internal/storage/concurrency_test.go b/internal/storage/concurrency_test.go new file mode 100644 index 0000000..51e1f11 --- /dev/null +++ b/internal/storage/concurrency_test.go @@ -0,0 +1,62 @@ +package storage + +import ( + "fmt" + "path/filepath" + "sync" + "testing" + + "github.com/sandrodach/mindsift/internal/domain" +) + +// Many writers at once on one SQLite file (FLAW-23): transactions that read before they write must +// wait for the write lock instead of failing with SQLITE_BUSY. +func TestConcurrentWritersDoNotFail(t *testing.T) { + store, err := NewStore(filepath.Join(t.TempDir(), "load.db")) + if err != nil { + t.Fatal(err) + } + defer store.Close() + u, _ := store.CreateUser("load@example.com", "hash", "Last", "member") + tl, _ := store.CreateTaskList(u.ID, "", "", "Last", "", "") + var ids []string + for i := 0; i < 20; i++ { + th, _ := store.CreateThoughtWithDetails(u.ID, fmt.Sprintf("Aufgabe %d", i), "normal", "manual", "", tl.ID, "todo", "") + _, _ = store.UpdateThoughtTriage(u.ID, th.ID, "keep", "task", "", "", tl.ID) + ids = append(ids, th.ID) + } + + var wg sync.WaitGroup + errs := make(chan error, 400) + for w := 0; w < 16; w++ { + wg.Add(1) + go func(w int) { + defer wg.Done() + for i := 0; i < 15; i++ { + status := "todo" + if (w+i)%2 == 0 { + status = "done" + } + items := []domain.ThoughtReorderItem{{ID: ids[(w+i)%len(ids)], SortOrder: i, KanbanStatus: &status}} + if err := store.ReorderTasks(tl.ID, items); err != nil { + errs <- fmt.Errorf("reorder: %w", err) + } + if _, err := store.CreateThought(u.ID, fmt.Sprintf("Gedanke %d-%d", w, i), "normal", "manual", ""); err != nil { + errs <- fmt.Errorf("create: %w", err) + } + } + }(w) + } + wg.Wait() + close(errs) + failed := 0 + for err := range errs { + if failed < 3 { + t.Error(err) + } + failed++ + } + if failed > 0 { + t.Fatalf("%d of %d writes failed", failed, 16*15*2) + } +} diff --git a/internal/storage/sqlite.go b/internal/storage/sqlite.go index 8242198..b9c2639 100644 --- a/internal/storage/sqlite.go +++ b/internal/storage/sqlite.go @@ -32,9 +32,12 @@ func NewStore(dbPath string) (*Store, error) { } } + // _txlock=immediate: transactions take the write lock when they begin. A transaction that reads + // first and writes later would otherwise fail at once with SQLITE_BUSY while another one writes + // (busy_timeout cannot help there); now it simply waits its turn (FLAW-23). dsn := dbPath if !strings.Contains(dsn, "?") { - dsn += "?_pragma=busy_timeout(10000)&_pragma=journal_mode(WAL)&_pragma=synchronous(NORMAL)&_pragma=foreign_keys(1)" + dsn += "?_pragma=busy_timeout(10000)&_pragma=journal_mode(WAL)&_pragma=synchronous(NORMAL)&_pragma=foreign_keys(1)&_txlock=immediate" } db, err := sql.Open("sqlite", dsn)