chadtmiller.com/knotless

Switch repository

Loading…

What changes

A check's log is read ahead. When a check record with a log comes from the network, and its repository names the check's runner, the server reads the log's tail from the runner's PDS in the background, at most four at once. One past those is read when its page is looked at, as before. Private repositories' checks don't come this way, and their tails are still read on each look.

Many more tails are kept. Log tails get a cache of their own, weighed in bytes: 64 MiB of them, where tails and pictures shared 64 entries before. Pictures and attachments keep their 64-entry cache, so a bigger cache never holds thousands of whole images. card::Newest learns to weigh its values (Newest::weighed); Newest::new counts them as before.

Why

On prod a check page's first look waited 250–380 ms longer than a second one (first 0.80–0.95 s, again 0.55–0.64 s, measured from a 180 ms round trip): the time to fetch the log blob from the runner's PDS. With about 500 checks a day and 64 entries shared with pictures, a tail was gone within hours, so most first looks missed. A tail is a few KB most often and 256 KiB at most, so a byte budget keeps thousands of them.

How it was checked

• Unit tests: Newest::weighed replaces, drops the oldest past its weight, and always keeps the newest; log_to_warm reads ahead only a log of a runner the repository names, and nothing for a check with no log, or no check.
• The serve tests that file checks through the firehose (tests/web.rs's check page) run the read-ahead path; they assert the page as before, not the read-ahead itself.
• Left to CI: build, clippy, the coverage gate.
• After deploy: time getCheck on a fresh check, first look against second.

Shortcomings

A check filed during a rollout, missed by the stream, is not read ahead; its page reads it as it did. A burst of more than four finishing at once reads ahead only those four.

This reads each check's log tail ahead as the check record arrives, at most four at once. It also gives log tails their own byte-weighted cache, 64 MiB. I read the whole diff and found nothing that must change.

• Newest::put accounting: the weight stays consistent on replace, eviction and the heavier-than-cap single entry. new keeps its old behaviour, since every entry weighs 1 and order.len() > 1 is implied once the weight passes the cap.
• Cache keys: the warm path and get_check both use log/{runner did}/{cid} through run_blob, so a warmed tail is found on the first look.
• Private repositories: log_to_warm doesn't check visibility. A private repository's checks should not be in the public index, which is what keeps them out of this path. If one ever was, run_blob with space: None would cache its tail. get_check would never read that entry for a private repository, so nothing would leak. Say so if you'd rather guard it explicitly.
• Burst behaviour: try_acquire drops the surplus checks, as the body says, and get_check still reads those on demand.

I didn't see the check results, and the body leaves build, clippy and the coverage gate to CI.

Since my earlier approval the head moved by one commit of the pull's own ("has no branch a test never takes") plus a merge of main. I re-read the full diff and found nothing that must change.

The last commit tidies the read-ahead path. warm_check_log now takes the permit only once there is a CID to read, and get_check shares shown_tail with the warm path. Both go through run_blob with the key log/{runner did}/{cid}, so a warmed tail is found on the first look. Newest::weighed keeps its weight consistent on replace, on eviction and for a single entry heavier than the cap. The tests cover that and the runner check in log_to_warm. docs/design/serve.md is updated to match.

I didn't see the check results. The body leaves build, clippy and the coverage gate to CI.