Skip to content

Windows CI: multi-process SQLite init contention fails a different test most runs #121

Description

@hristo2612

Splitting this out of #110, where @e1010101 diagnosed the mechanism. Filing it because it has now blocked or muddied three merges in one day, and because #110 is explicitly an interim.

Symptom

The unit-tests (windows-latest) leg fails with SqliteError: attempt to write a readonly database, thrown from inside initDb() under runSqliteBusyRetry. A different test fails each time, which is what makes it read as unrelated noise rather than one bug:

Run Failing test
PR #105 sessions/__tests__/callback-concurrent-init.test.ts — "serializes concurrent opens and one transactional migration"
PR #118 work-items/__tests__/migrate.test.ts — "lets 32 processes allocate distinct monotonic ids without reuse"

Both are multi-process tests (16 and 32 workers) that race to open and migrate one registry. The common factor is not the test, it is initDb() under concurrency.

Mechanism

Established in #110 and not in dispute: journal_mode = WAL takes a brief exclusive lock to rewrite the header, and when a peer holds the file at that instant SQLite reports SQLITE_READONLY rather than SQLITE_BUSY. #104 taught isTransientSqliteError to treat READONLY as transient, so the retry wrapper does engage. It just runs out of ladder: [10, 50, 200, 500, 1000] spends 1.76 s, while an instrumented run measured a dead worker that had contended for 3.5 s. At 6x CI concurrency @e1010101 recorded RETRY-GAVEUP elapsed=15015ms.

Why a bigger budget is not the fix

#110 raises the ceiling to 15 s and is worth taking on its own merits, but its author said the important part plainly: it raises a ceiling rather than removing one, and no fixed budget suffices for unbounded contention. He also declined to claim a failure-rate improvement off 1/12 vs 2/12, which was the right call.

The cost of relying on the budget is real. Every process that loses the race burns its full budget in a synchronous Atomics.wait, so N contending processes serialize into N × budget of blocked wall-clock. On a real machine that is a gateway that appears hung with nothing logged.

What this costs us today

#104 made the Windows leg unconditional precisely so it would be trustworthy. A leg that goes red on a different unrelated test most runs trains everyone to merge through it, which is exactly the failure mode #104's own PR body warned about. I have now merged two PRs on a red Windows leg after establishing ownership by hand each time. That does not scale, and eventually it hides a real regression.

The actual fix

Serialize schema init across processes rather than retrying into it. Roughly:

  • A lock file next to the registry, acquired before initDb() opens the database and released after migration completes.
  • Losers wait on the lock rather than racing the WAL header, so the retry ladder becomes a backstop for genuine contention instead of the primary mechanism.
  • The lock must be crash-safe: a process killed mid-migration must not wedge every future boot. Staleness detection by pid liveness plus an mtime bound, not by timeout alone.
  • Keep runSqliteBusyRetry as-is underneath. This reduces how often it fires; it does not replace it.

Acceptance

  • callback-concurrent-init and work-items/migrate pass on windows-latest across 10 consecutive runs.
  • A test that kills a worker mid-migration and asserts the next process still boots.
  • No increase in single-process cold-start time. Measure it, do not assume it.

@e1010101 — you diagnosed this and flagged the lock file as the real fix yourself, so it is yours if you want it. No obligation; say so either way and I will pick it up otherwise. #110 is still worth merging independently and I would like to land it first.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions