chadtmiller.com/knotless

Switch repository

Loading…

What changes

Reads get connections of their own. The index wrote and read through one SQLite connection behind one mutex. It now writes through that one and reads through up to eight read-only connections (PRAGMA query_only). Each is opened the first time a read needs it and kept for the next read. The database is already in WAL mode, so a read waits on no write and on no other read, only for a free connection once eight are busy. A reader that cannot be opened reads through the writer instead. The 75 methods that only read take a reader. The 36 that write, or read inside a write, keep the writer.

Forgetting an account is narrower. Index::purge stamped every repository's cached pulls and issues as changed. It now stamps the repositories the account owned, moved from or to, wrote about or ran checks on. It still stamps everything when the account owned a space, whose records can be about any repository. check_run gains an index on author, so purge's check-run delete stops scanning it. The mention delete takes the account's URI range by index instead of substr.

Why

On prod, five requests for one repository page (getRepo, listTags, listBranches, getTree, getLastCommits) finished together at about 1.65s, and none of them ran anything slow. They were queued on the index's mutex. Forgetting a deleted account happens about 25 times a day. Its check-run delete took 377ms on average and up to 2.5s, holding that mutex. Then every repository's cache was emptied, so the next page of each read and assembled its records again: the forge read logged as slow 149 times in three days.

How it was checked

Unit tests: a_read_waits_for_a_reader_to_be_free takes every reader, shows a ninth read waits until one is given back, and shows a reader cannot write. pulls_and_issues_are_kept_until_written_about now shows that forgetting an account that never wrote about a repository leaves its cache standing, and that forgetting its issue's author does not. The rest is left to CI: the coverage gate, and the plans of the new purge statements.

A first version opened all eight for every index at once. The run, triage and review tests, which keep many indexes open in one process, then ran out of file descriptors, and SQLite failed reads with a disk I/O error (1802). Opening readers only as reads need them fixed it.

Shortcomings

Reads still run on tokio's worker threads, so a slow statement holds a worker. They no longer hold each other up, though. Moving them to spawn_blocking would mean changing every caller.

🤖 Generated with Claude Code

This gives the index eight read-only WAL connections (PRAGMA query_only) besides the one writer. It moves the read-only methods onto them. purge now stamps only the repositories the account owned, moved from or to, wrote about or ran checks on, and stamps everything only if it owned a space. It also adds a check_author index and a range delete on mention.

I read the whole diff and found nothing that must change.

• Readers: The Reader guard returns its connection on drop, and take loops on the condvar, so a connection can't be lost or missed. reader() sets up what connect does for reads: zone_day, the busy timeout and the tracing. No reader method uses the writer's temp purged table. A reader that opens after a commit sees that commit, so a write followed by a read still agrees.
• Purge range: at://{did}0 is the first string after the at://{did}/ prefix, since / is 0x2F and 0 is 0x30. The range matches what substr matched.
• Purge scopes: The scopes are read before the deletes and stamped after the commit, so the stamps follow the data change. check_author is created outside the rebuilt branch, so existing prod databases get it without a VERSION bump.
• Docs: docs/design/serve.md is updated.

CI had not finished when I looked. The check and test runs were queued or running, and the author left the plans of the new statements to test. Please check that run. The statements to watch are the moved and forge scope selects, and the mention delete, which joins its range to did = ?1 OR uri IN purged and needs an index to avoid a scan. If the plan check flags one, add an index or a -- scans: note.

The only change since my last round is dropping an unused PathBuf import in index.rs. That is a harmless cleanup, and I still found nothing that must change.

CI had not finished on this commit. check, test and e2e were still queued when I looked. Before you merge, check that test passes. Look in particular at the plans for the new purge statements (the moved and forge scope selects, and the mention range delete). If the plan check flags one as a scan, add an index or a -- scans: note.

The latest commit opens readers on demand instead of all eight at once, so many indexes in one process no longer run out of file descriptors. I found nothing that must change.

• Lock order: take and Reader::drop both lock open and then free, so the order is consistent. A returned connection is pushed while open is held. A waiter on the condvar therefore can't miss it.
• Opening: *open only counts a reader after it opened. A failed open doesn't leak a slot.
• Fallback: a reader that can't be opened reads through the writer. The new test covers this with a path that doesn't exist.
• Writer-lock wait: in that fallback, writer.lock() runs while open is held. If the writer is busy, every other take waits too, even for a free reader. This only happens when opening fails, so it isn't worth changing now.

check and test are still queued on this commit, so I could not confirm they pass. Before you merge, check that test passes. Look in particular at the plans for the new purge statements, the moved and forge scope selects and the mention range delete. If one is flagged as a scan, add an index or a -- scans: note.