feat(pipeline): generate photos concurrently, and report what a reel burned - #55
Conversation
…burned
Five photos could never finish. Live generation was one call per photo run
strictly one after another, so a five-photo reel was about 26 minutes
against a 12-minute waiting window. The owner wants a five-photo cap, so
the constraint had to go, and it was ours rather than the model's: the
calls were always independent. One photo in, one clip out, and a chapter
bridge is generated from the neighbouring chapters' photos, never from a
generated clip, so nothing in a reel waits on anything else in it.
They now run concurrently, five at a time. A reel is one wave rather than
one call per photo: 359s instead of 1615s at the cap, about 6 minutes
instead of about 27.
Generation and storage are now separate phases, which is what makes that
safe. Calls overlap in a bounded pool; every artifact is then written and
sealed on one thread in plan order. The write sequence, the edit and the
sealed manifest are identical to a fully sequential run whatever order the
calls finish in, and a test proves it by running the same reel at
concurrency 1 and 5 and comparing the sealed steps. Another runs a
provider that deliberately finishes in reverse order.
The provider's real concurrency limit is not published to us and is not
worth discovering by firing paid calls at it, so five is a chosen
conservative number, not a tuned one, and it is not hoped away. A
rate-limited call backs off, jittered, and retries within a bounded
budget. Only a rate limit is retried: a dead balance or a rejected request
fails the same way however many times it is asked, only slower, and for
credit only after burning more of the thing that ran out. One failure no
longer waits for the rest of the wave, so the honest fallback still
arrives promptly. CINEMORY_MAX_CONCURRENT_GENERATIONS=1 restores the old
behaviour without a deploy.
The classifier that decides all that is the same one the API uses to tell
a visitor why a reel fell back, moved to its own module so the two
readers cannot drift into disagreeing about what a failure was.
Usage accounting. Every run now reports what it burned, readable per job
long after the fact rather than only in logs: provider calls broken down
by model, wall clock per call and for the run, provider seconds summed
across calls (which diverges from wall clock by exactly what concurrency
saved), bytes to and from the provider, and objects written to storage,
which is the number that predicts the Backblaze Class B ceiling we reached
75% of. It comes back inside result.usage from GET /reels/jobs/{job_id},
and goes out as one greppable log line.
There is no money in it. Reliable per-call pricing for these models is not
available to us, and an invented euro figure would look authoritative and
be wrong, so it reports units burned. A test asserts no currency ever
appears. It rides on the result rather than inside the sealed manifest:
the manifest already seals the per-step facts this is built from, and
putting mutable bookkeeping inside a hashed artifact would change the
manifest hash of every reel ever made for a reason unrelated to
provenance.
The cap is now 5 and the estimate follows the wave arithmetic. The cap is
a decision rather than a derivation, so capFitsTheWindow() is the check
that it still fits, and a cross-language contract test pins the
concurrency the estimate assumes to the concurrency the backend runs.
Also closes a gap found on the deployed app: pasting a reel link into a
tab that already had Cinemory open changed only the fragment, so nothing
happened until the visitor thought to reload. The route is now applied on
hashchange too, while the skip link's own #main-content stays inert.
The half of the concurrency change with real risk is the provider, not storage: five threads now call one adapter instance. Cinemory's own fakes cannot show anything about it, being stateless per call or lock-protected, so this drives the real genblaze Pipeline with five genuinely overlapping calls held at a barrier. Nothing is shared that could corrupt a result. The Pipeline is a local per call, and in a live deployment so are the provider and the storage backend (the injection seams are None in production); this test is harsher than production because it shares one injected provider object across every thread. Every call verifies its own asset's sealed sha256 against the bytes it returns, so a thread picking up another's asset would raise rather than pass quietly. The one piece of per-call instance state, last_manifest, is documented rather than defended: nothing in src reads it, an attribute assignment is atomic so it is always one real manifest, and after a wave it holds whichever call finished last, which is now stated on the attribute instead of implied by its name.
Provider thread safety, checked rather than assumedThe concurrency change parallelises calls to Audited The one exception is New test: Coverage after: pytest 97.33% (gate 90). |
1 participant
Follow-up to #54. The owner wants a 5-photo cap; that could not work as the code stood, so this makes it work and adds the usage accounting asked for alongside it.
1. Five photos could never finish, and the constraint was ours
Live generation was one call per photo, run strictly one after another (
pipeline.py: a plain nestedfor chapter: for photo: self._step(...), each call blocking). Measured live today on the deployed service, sequentially:So a call is ~310 s and fixed overhead ~15 s. Five photos sequentially is ~26 minutes against a 12-minute waiting window.
The calls were always independent: one photo in, one clip out, and a chapter bridge is generated from the neighbouring chapters' photos (
frm.photos[-1],to.photos[0]), never from a generated clip. Nothing in a reel waits on anything else in it. Running them one after another was costing the full sum of their latencies for no reason.They now run concurrently, bounded:
2. Nothing else may move
Generation and storage are now separate phases, which is what makes the concurrency safe rather than merely fast:
_generateis the remote call. No storage, no shared state, safe in a worker thread._recordwrites the artifact and seals the step. Main thread, in plan order.So the write sequence, the edit and the sealed manifest are identical to a fully sequential run whatever order the calls happen to finish in. Storage concurrency semantics are untouched: B2Storage still sees exactly the same serial sequence of
puts it always did, including its index read-modify-write.Pinned by tests, not by assertion in prose:
3. Rate limiting, honestly
We do not know GMI's concurrency limit. It is not published to us and it is not worth discovering by firing paid calls at it until it complains. So
MAX_CONCURRENT_GENERATIONS = 5is a chosen conservative number, not a tuned one: it is set to the photo cap, so one reel is at most one wave and a second concurrent reel doubles it rather than multiplying it. Five simultaneous calls is small in absolute terms for a hosted inference API.It is not hoped away either. There was no 429 handling anywhere before this:
shutdown(wait=False, cancel_futures=True)), so the honest fallback from feat(frontend): make a reel survive the tab, and say how long it takes #54 still arrives promptly instead of after every other multi-minute render finishes. Tested with a 2 s stall: fails in under 1.5 s;CINEMORY_MAX_CONCURRENT_GENERATIONS=1restores exactly the old sequential behaviour without a deploy.The classifier deciding what is retryable is the same one the API uses to tell a visitor why a reel fell back, moved into
src/cinemory/failures.pyso the two readers cannot drift into disagreeing about what a failure was.4. Usage accounting, per job
GET /reels/jobs/{job_id}now returnsresult.usage:provider_calls,provider_calls_by_modelduration_msprovider_secondscalls[]input_bytes_to_provider,output_bytes_from_providerobjects_written,bytes_writtenmax_concurrencyThere is no money in it. Reliable per-call pricing for these models is not available to us, and an invented euro figure would look authoritative and be wrong. It reports units burned. A test asserts no currency symbol, rate or cost field ever appears.
The same numbers also go out as one greppable log line per run.
Why not inside the sealed manifest. The manifest already seals the per-step facts this is built from: provider, model,
started_at,finished_at, outputsize_bytes, for every step. Those are provenance and they do not move. This rollup is observability, derived from those same sealed records plus counters kept during the run, and it rides on the run's result, which is what gets stored in the job status. Putting mutable bookkeeping inside a hashed artifact would change the manifest hash of every reel ever made for a reason that has nothing to do with provenance, and would invalidate every existing sealed reel.attemptsper call is included deliberately: it is the difference between "the provider is fine" and "we are at its limit and only just getting away with it".5. The cap, and a gap found on the deployed app
The cap is now 5, and it is a product decision, not a derivation (the window would allow more at this concurrency). So
capFitsTheWindow()is the check that it still fits, and a test fails if the cap rises, the concurrency drops, or the measurement gets worse. The estimate follows the wave arithmetic, so 1 and 5 photos both read "about 6 minutes" while the sentence still agrees in number.SAMPLE_PHOTO_COUNTfollows the cap, so the one-click demo path is back to a full five-frame storyboard.A cross-language contract test (
tests/integration/test_budget_contract.py) readsreel-budget.tsand pins the concurrency the frontend estimate assumes topipeline.MAX_CONCURRENT_GENERATIONS. That pair lives in two languages that cannot import each other and is exactly the kind that silently drifts: someone lowers the backend concurrency for a good reason and the app carries on promising six minutes for a reel that now takes half an hour.Found by checking the live app rather than the tests: pasting a reel link into a tab that already had Cinemory open changed only the fragment, so the browser navigated nothing and the pasted link sat there doing nothing until the visitor thought to reload. The route is now applied on
hashchangetoo. The skip link's own#main-contentstays deliberately inert, so pressing Tab never tears down the reel someone is watching.Verified on the deployed app (build
c584df8, from #54)#reel/RL4DDZNFJmx6Hs5ZA62rk_0Con the deployed app landed directly on "Rolling..." with "Picking this reel back up. It carried on without the page open." and "Already in progress" rather than a false "0 photos".#reel/nope!showed "We could not find that reel" with a fresh start, no request and no spinner.provider: genblaze,provider_degraded: false.Tests
New:
tests/integration/test_pipeline_concurrency.py(12: overlap, bound, reverse-finish ordering, identical sealed manifest at concurrency 1 vs 5, no pool for a single call, the off switch, rate-limit retry and its budget, no retry for credit/refused/rejected, fail-fast),tests/integration/test_usage_accounting.py(9, including the no-money assertion and reading usage back off a real job poll),tests/unit/test_failures.py(18),tests/integration/test_budget_contract.py(3). Extended: three App specs for the pasted-link path, and the budget suite rewritten around waves.Green locally: 340 vitest (99.1% lines / 92.2% branches, gate 90/85), 25 Playwright including axe and 375px, full pytest 97.3% (gate 90), ruff clean, readiness gate PASS (100% automatable).
demo/untouched.