chadtmiller.com/knotless

Switch repository

Loading…

What changes

Notifications are fast again for owners. In Index::notified, the query behind listNotifications, getUnreadCount and the push loop:

• The block and hide check in the watching and mention branches now says instr(o.uri, '/dev.knotless.block/') OR instr(o.uri, '/dev.knotless.hide/') on its own, so SQLite searches the partial index forge_moderation (only the few blocks and hides) instead of forge_repo_created by (repo_did, repo_key), which is every record of the repository, JSON parsed, for each row heard of.
• subs is AS MATERIALIZED: inlined, its json_each(?7) was parsed again for each candidate row in the outer mute/ignore filter and in the watching branch.

The rows found are the same.

Why

Since An owner hears all of its own repository until it chooses otherwise deployed (03:35 UTC on Nov 8), every owner gets an all watch on each of their repositories. The query itself was unchanged, but that turned on the watching branch for the busiest repositories. Each record written to them since OWNED_SINCE (every pull, comment, state and queue record) ran the block and hide subquery, which walked the whole repository's forge rows. Before, almost no one watched a repository, so the cost never showed. On prod, getUnreadCount went from about 16 requests a day over 500 ms to 434 today, and the statement's mean went from about 143 ms to 572 ms (admin_stats). Root +layout.ts waits for it, so every logged-in page waits too: page traces show about 150 ms of work, then about 600 to 800 ms of this one statement.

It also grows: the rows since OWNED_SINCE pile up daily, and each one costs a scan of the repository.

The partial index was already there for exactly this. The subquery's OR of ANDs just never implied its WHERE, so SQLite could not use it.

How it was checked

A spike outside the tree: the index's schema and this query taken from index.rs as written, and a synthetic index the size of chadtmiller.com/knotless (16 owned repositories, 22k forge rows, 9k comments, 18k check runs, 314 records since OWNED_SINCE). Notified for the owner, with no ANALYZE, as on prod, the best of 3 runs:

For each case, the rows found before and after are identical. That includes an index with a hide and a block written in the watched repository. The plan shows MATERIALIZE subs and both subqueries on SEARCH o USING INDEX forge_moderation (repo_did=? AND repo_key=?). The existing notification tests, which cover hides and blocks heard of as watching and as mentions, run in CI.

Shortcomings

harness/plans.mjs did not catch this: the plan is a SEARCH, not a SCAN, yet it visits a whole repository's rows for every outer row. Flagging a correlated search of a growing table by a repository prefix is left for another pull.

OWNED_SINCE still lets the watching rows grow without bound. They are now cheap per row, but they are still all found before the page's LIMIT.

This pull adds instr(o.uri, '/dev.knotless.block/') OR instr(o.uri, '/dev.knotless.hide/') to the block and hide NOT EXISTS subqueries in the watching and mention branches of Index::notified. It also makes subs AS MATERIALIZED. I read the diff and the forge_moderation definition. I did not run the query or check the pull's runs.

• Same rows: The added clause is implied by the existing (block AND …) OR (hide AND …), so it cannot change which rows match.
• Index use: The clause is textually identical to the WHERE of the partial index forge_moderation, which is what lets SQLite choose that index.
• MATERIALIZED: It only changes when json_each(?7) is evaluated, not what it returns.

No correctness issues found. The shortcomings section is honest about what is left. harness/plans.mjs and the unbounded OWNED_SINCE rows are left for later, which is fine.