kayg/calternal

Watch

Fork 0

PERF: cached HLS reads serialize globally while awaiting rendition access writes #783

New issue

Open

opened 2026-10-02 13:10:38 +00:00 by kayg · 2 comments

kayg commented

2026-10-02 13:10:38 +00:00

Owner

Found in the read-mostly server architecture audit #663. Applies DESIGN §58 rules 1, 2 and 8 from queued job/instant-663. Source base origin/dev = c4a61e8cf090170f35b1bed3350d9de20c83ecd5; pending origin/job/merge-round-7a = 2f4482ded066d9c5d9c59130377907f7fd2916c9. This is structural evidence, not a measured latency or confirmed security blocker.

Context: playlist and segment GETs occur throughout video playback in Files and Photos. The source byte route streams with ETag/range support; the cached HLS path has a separate bottleneck. #446 covers physical projection separation, not read scheduling.

Evidence:

crates/plugins/video/src/routes.rs:291–329: serve_cache_file takes state.cache_lock, opens the immutable cached file, then awaits touch_rendition before it returns the stream.
routes.rs:110–122: master_playlist uses the same global cache_lock and awaits access writes for each available height. Variant and segment routes also enter serve_cache_file.
transcode.rs:139–153: touch_rendition UPDATEs last_accessed through the single SQLite writer pool on every read.
This lock and awaited UPDATE remain at the same route lines on round 7a; its transcode changes do not remove them.

Reasoned impact: cached video response setup is delayed by unrelated Index writes. Because the cache lock is shared, one such wait queues other hashes/heights/Users behind it. The lock is released before body streaming; this finding does not claim streaming itself holds the lock. A write failure can fail a GET whose cached bytes were already opened.

Concrete fix: open/pin an immutable rendition handle under the shortest required cache guard, then release the guard before any async wait. Coalesce last_accessed bookkeeping in a bounded background batch; eviction must account for pending touches or pinned readers. A healthy cached read must not need writer_pool. Keep current item/Share authorization before cache access, preserve transcode deduplication and eviction invariants, and coordinate with #446.

Regression tests: hold the SQLite writer and concurrently read warm segments from two different renditions/Users. Both should start streaming before writer release. Test concurrent cache eviction, revocation, background touch failure and immutable open-handle lifetime; reads must not expose unauthorized or partial bytes.

Validation for the fix: preserve existing assertions and protocol status codes. Run per-crate fmt/clippy/test, plus calternal-server if the route or provider contract changes. Extend an existing bench profile with warm/cold latency, CPU, RSS and a realistic large-data burst. Measure ≥5 samples on the perf VM under /root/perf.lock with load recorded inside the lock and HDD emulation; compare only a matching baseline. The audit itself did not run a server or benchmark. No product edit is requested from the audit branch.

Found in the read-mostly server architecture audit #663. Applies DESIGN §58 rules 1, 2 and 8 from queued `job/instant-663`. Source base `origin/dev` = `c4a61e8cf090170f35b1bed3350d9de20c83ecd5`; pending `origin/job/merge-round-7a` = `2f4482ded066d9c5d9c59130377907f7fd2916c9`. This is structural evidence, not a measured latency or confirmed security blocker. Context: playlist and segment GETs occur throughout video playback in Files and Photos. The source byte route streams with ETag/range support; the cached HLS path has a separate bottleneck. #446 covers physical projection separation, not read scheduling. Evidence: - `crates/plugins/video/src/routes.rs:291–329`: serve_cache_file takes state.cache_lock, opens the immutable cached file, then awaits touch_rendition before it returns the stream. - `routes.rs:110–122`: master_playlist uses the same global cache_lock and awaits access writes for each available height. Variant and segment routes also enter serve_cache_file. - `transcode.rs:139–153`: touch_rendition UPDATEs last_accessed through the single SQLite writer pool on every read. - This lock and awaited UPDATE remain at the same route lines on round 7a; its transcode changes do not remove them. Reasoned impact: cached video response setup is delayed by unrelated Index writes. Because the cache lock is shared, one such wait queues other hashes/heights/Users behind it. The lock is released before body streaming; this finding does not claim streaming itself holds the lock. A write failure can fail a GET whose cached bytes were already opened. Concrete fix: open/pin an immutable rendition handle under the shortest required cache guard, then release the guard before any async wait. Coalesce last_accessed bookkeeping in a bounded background batch; eviction must account for pending touches or pinned readers. A healthy cached read must not need writer_pool. Keep current item/Share authorization before cache access, preserve transcode deduplication and eviction invariants, and coordinate with #446. Regression tests: hold the SQLite writer and concurrently read warm segments from two different renditions/Users. Both should start streaming before writer release. Test concurrent cache eviction, revocation, background touch failure and immutable open-handle lifetime; reads must not expose unauthorized or partial bytes. Validation for the fix: preserve existing assertions and protocol status codes. Run per-crate fmt/clippy/test, plus calternal-server if the route or provider contract changes. Extend an existing bench profile with warm/cold latency, CPU, RSS and a realistic large-data burst. Measure ≥5 samples on the perf VM under /root/perf.lock with load recorded inside the lock and HDD emulation; compare only a matching baseline. The audit itself did not run a server or benchmark. No product edit is requested from the audit branch.

kayg referenced this issue

2026-10-02 13:12:21 +00:00

PERF: instant everywhere — app-wide speed architecture rules, audit and shared primitives #663

kayg referenced this issue

2026-10-02 13:19:51 +00:00

PERF: instant everywhere — app-wide speed architecture rules, audit and shared primitives #663

kayg commented

2026-10-02 13:25:54 +00:00

Author

Owner

Starting work on job/webperf, based on 2f4482ded066d9c5d9c59130377907f7fd2916c9 (job/merge-round-7a). I am reading the matching audit evidence and will report the concrete finding, regression coverage, measurements, and gate output here when finished.

Starting work on `job/webperf`, based on `2f4482ded066d9c5d9c59130377907f7fd2916c9` (`job/merge-round-7a`). I am reading the matching audit evidence and will report the concrete finding, regression coverage, measurements, and gate output here when finished.

kayg commented

2026-10-02 17:28:13 +00:00

Author

Owner

Finding: HLS playlist/file reads awaited last_accessed updates on the single Index writer while holding the cache eviction lock. Reads now drop the lock after opening the confined file and queue at most one disposable best-effort access update. Added a regression test that holds the writer connection during a playlist response. Rust gates are still in progress.

Finding: HLS playlist/file reads awaited `last_accessed` updates on the single Index writer while holding the cache eviction lock. Reads now drop the lock after opening the confined file and queue at most one disposable best-effort access update. Added a regression test that holds the writer connection during a playlist response. Rust gates are still in progress.

kayg referenced this issue from a commit

2026-10-02 17:39:10 +00:00

Make HLS cache access hints best effort (#783)

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

kayg/calternal#783

No description provided.

Rows
Columns