Bound media request queues, filmstrip cache keys, and export job sizes - #627
Open
badbread wants to merge 5 commits into
Open
Bound media request queues, filmstrip cache keys, and export job sizes#627badbread wants to merge 5 commits into
badbread wants to merge 5 commits into
Conversation
…t job sizes Media routes could queue without limit. Every expensive media path (clip and low-bitrate transcodes, filmstrip frame extraction, the live camera still proxy) waits on a shared work semaphore, and those waits were open-ended: once the permits were gone a request parked indefinitely, holding its connection and never answering. The media router also had neither a request timeout nor a rate limit, unlike the JSON routes. Changes: - New services/api/src/media_limits.rs: one helper for every media semaphore acquire, with a per-call-site wait budget. A saturated queue now answers 503 with Retry-After instead of hanging. Applied to the clip transcode, clip thumbnail, low-bitrate transcode, filmstrip extraction, and still proxy. - Media router gets its own (much larger) rate-limit bucket, burst 1200 and 240/s sustained, roughly 6x the busiest real client, plus a 180 s time-to-response bound. tower_http's TimeoutLayer bounds only the handler future, not the response body, so segment downloads, export archives, and the live stream are unaffected. The on-demand size-verification walk is mounted outside the timeout because it can legitimately run for minutes. - Filmstrip widths clamp and then snap to 80/160/320/480/640 before entering the cache key, so one instant has five renditions instead of 593. The buckets keep every width the shipped clients ask for exact, so no warm or pre-generated cache entry is invalidated. The frame route's stale "width is unused" doc comment and dead_code attribute are corrected: it has been honoured since the width became part of the cache key. - POST /export sorts and de-duplicates its camera list (the RBAC scope filter never did, so a repeated camera ran one full-range encode per repeat, all writing the same output file), caps it at 50 distinct cameras to match the batch route's clip cap, and caps the window with a new EXPORT_MAX_RANGE_SECONDS (default 86400). The same range cap applies per clip in a batch export. - Export ffmpeg children get kill_on_drop plus a wall-clock budget derived from the range (4x realtime, floored at 10 min, capped at 24 h). A wedged child now fails the job with a stored error instead of leaving it Running forever and permanently consuming an EXPORT_MAX_CONCURRENT slot. - The export byte-budget sweeper counts in-flight jobs' bytes against the budget (it still never deletes them), so the total no longer sits above EXPORT_CACHE_MAX_BYTES for as long as jobs keep running. - GET /cameras/:id/frame.jpg goes behind a new frame-proxy semaphore (FRAME_PROXY_MAX_CONCURRENCY, one per core clamped 8..32). A camera that is disabled, rejected by go2rtc, or whose last fetch failed is latched for 10 s and served by a single quick attempt, so a wall of tiles pointed at a dead camera fails in about a second per poll instead of ~17 s. Operator-visible: three env keys documented (EXPORT_MAX_RANGE_SECONDS and FRAME_PROXY_MAX_CONCURRENCY are new, EXPORT_MAX_CONCURRENT already existed but was undocumented and unforwarded). .env.example, docker-compose.yml, and the docs-site environment reference updated; two docs/DECISIONS.md entries added. Tests: unit tests for the width quantizer, the export range/count/dedup validation, the derived ffmpeg budget, the timeout-kills-and-reaps path, and the bounded-acquire helper (a zero-permit semaphore must take the 503 path within its budget); a new services/api/tests/export_limits.rs integration test proves a duplicated camera list yields one job entry per distinct camera and that an over-range request is 400 before any job is allocated. Signed-off-by: badbread <badbread@users.noreply.github.com>
Signed-off-by: badbread <badbread@users.noreply.github.com>
Signed-off-by: badbread <badbread@users.noreply.github.com>
Signed-off-by: badbread <badbread@users.noreply.github.com>
Signed-off-by: badbread <badbread@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
The media routes could queue work without limit. Every expensive media path (clip and low-bitrate transcodes, filmstrip frame extraction, the live camera still proxy) waits on a shared work semaphore, and those waits were open-ended: once the permits were gone, a request parked indefinitely, held its connection, and never answered. The media router also carried neither a request timeout nor a rate limit, unlike the JSON routes.
Alongside that, two paths let one request ask for an unbounded amount of work: the filmstrip cache key included an unquantized width, and
POST /exportcapped neither the time range nor the camera list.Bounded waits on the media router
services/api/src/media_limits.rs. One helper (acquire_bounded) wraps every media semaphore acquire with a per-call-site wait budget. A saturated queue now answers503withRetry-Afterinstead of hanging. Applied to the clip transcode, the clip thumbnail, the low-bitrate segment transcode, filmstrip extraction, and the still proxy. Budgets: 15 s for the transcodes (which run in seconds in practice), 5 s for a single-frame extraction and for a still (a scrub thumbnail that arrives after 5 s is off screen; a wall tile polls again in about a second).rate_limit::RateLimiterand keyed exactly as the JSON bucket is: burst 1200, 240/s sustained. The busiest real client (a 16-tile low-bandwidth wall at 1 Hz, plus segment fetches, plus a scrub burst) sits near 30 to 40 req/s, so that is roughly a 6x margin, while a runaway loop is still stopped well before it can queue work.tower_http'sTimeoutLayerbounds only the handler future, not the response body (body deadlines are a separateResponseBodyTimeoutLayerthat is deliberately not used), so segment downloads, export archives, and the open-ended livestream.mp4cannot be cut: they return headers immediately and stream afterwards. The one route that legitimately produces for minutes, the on-demand DB-vs-disk size verification walk, is mounted outside the timeout for that reason.Filmstrip cache keys
Widths clamp and then snap to
80/160/320/480/640before entering the cache key, so one instant produces five renditions instead of 593. The buckets are chosen so every width the shipped clients ask for lands on itself (160 for the Android and desktop scrub lists, 320 for the Android playback wall, 480 for the iOS scrub still, the desktop preview frame, and theTHUMB_PREGEN_WIDTHdefault), so no warm or pre-generated cache entry is invalidated. The frame route's stale "width is unused" doc comment anddead_codeattribute are corrected: the width has been honoured since it became part of the cache key.Export job sizes
POST /exportsorts and de-duplicates its camera list. The RBAC scope filter never did (for an admin it returns the list verbatim), so a body repeating one camera N times ran N sequential full-range encodes, all writing the same output file with-y, all counted as one job againstEXPORT_MAX_CONCURRENT. The misleading "filter still runs to dedup" comment is gone./export/batchalready caps clips at: both routes fan out to one sequential encode per unit of work, so they get the same ceiling.EXPORT_MAX_RANGE_SECONDS(default 86400, a full day of one camera) caps the window with a clear 400. The same cap applies per clip in a batch export.kill_on_drop(true)(as the filmstrip and backup children already had) plus a wall-clock budget derived from the range: 4x realtime, floored at 10 minutes, capped at 24 hours. A wedged child now fails the job with a stored error instead of leaving itRunningforever, which used to consume anEXPORT_MAX_CONCURRENTslot permanently until the api was restarted.EXPORT_CACHE_MAX_BYTESfor as long as jobs keep running.Live still proxy
GET /cameras/:id/frame.jpggoes behind a new frame-proxy semaphore. A camera that is disabled, rejected by go2rtc, or whose last fetch failed is latched for 10 seconds and served by a single quick attempt, so a wall of tiles pointed at a camera that is down fails in about a second per poll instead of roughly 17. A successful fetch clears the latch immediately, so a camera that comes back is served normally on the next poll.Operator-visible changes
Three env keys are now documented and forwarded by compose. All have defaults; nothing is required.
EXPORT_MAX_RANGE_SECONDS86400FRAME_PROXY_MAX_CONCURRENCYEXPORT_MAX_CONCURRENT2Updated:
.env.example,docker-compose.yml,docs-site/docs/configuration/environment-reference.md.docs/AI-INSTALL.mdneeds no change: it does not enumerate optional tuning keys (same as the existingEXPORT_CACHE_MAX_BYTES,SEGMENT_LOW_CACHE_MAX_BYTES, andTHUMB_*keys). Twodocs/DECISIONS.mdentries record the bounded-wait choice and the quantizer plus export caps, with the alternatives rejected and revisit triggers.Behaviour a client can notice: under genuine saturation a scrub thumbnail, clip, or wall tile now fails fast with 503 plus
Retry-Afterinstead of eventually arriving. That is the intended trade, and the clients already treat a failed media fetch as "keep the placeholder and poll again". An export naming more than 50 distinct cameras, or a window longer than a day, is refused with a 400 that says why.How it was tested
Full gate on the build box against this branch:
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo test --workspaceagainst a throwaway Postgres 16, all green.New tests:
media_limits: a zero-permit semaphore must take the 503 path inside its budget (and not park), and a free semaphore hands out and returns a permit.filmstrip: the width quantizer keeps 160/320/480 exact, snaps to the nearest bucket, breaks ties deterministically downward, and collapses the whole legal input range onto exactly the five buckets.export: the range, distinct-camera, dedup, empty and inverted-range validation; the derived ffmpeg budget is floored, scaled and capped; a child that outlives its budget is killed, reaped, and reported asTimedOut.services/api/tests/export_limits.rs(new integration test, the real handler through the shared harness): a duplicated camera list produces one job entry per distinct camera, and an over-range request is 400 with no job allocated, while a window exactly at the cap is still accepted.Not verified by running: nothing here touches the clients, so no Android, iOS or Flutter build was needed. The saturation behaviour itself is covered by the unit test on the helper rather than by driving a real server to saturation.