Pipeline post-download work off the cycle's critical path#11
Open
jusii wants to merge 1 commit into
Open
Conversation
The sync worker used to run GPS extract + dashcam delete + mark_done inline on the same executor thread as the download. With Wi-Fi already saturated at N=1, every second spent on the post-download tail was a second of idle Wi-Fi between files. Add an opt-out PIPELINE_POST_DOWNLOAD setting (default on). When on, the tail runs on a dedicated single-thread executor so the worker can pick up the next pending file and start its download immediately. Validated on an A329: 4-7s of tail wall-clock per file moves off the critical path, saving ~6s × N files per cycle. Per-stage timing columns on download_queue (download_started_at, download_finished_at, tail_submitted_at, tail_finished_at) record ms timestamps so A/B benchmarking can attribute cycle time. DEBUG-level per-step logs cover the same ground for ad-hoc inspection. Test coverage: 4 new tests in test_sync_worker_pipeline.py pinning the tail-executor thread name, the inline fallback for A/B, end-of-cycle tail draining, and timing column population.
a21e637 to
9ea8ba3
Compare
Owner
|
Thanks for this. Agree in principle but I'll read through the changes before merging. Regarding viofosync.db in the recordings on an NFS share - perhaps it makes more sense to be in /config |
Contributor
Author
|
And just testing the sqlite theory, seems to be it atleast on my setup. Symlinked sqlite db from recordings dir which is in nfs to local filesystem, and that solved the stalling I was experiencing. So moving it to config dir would solve this. But anyhow, pipelining them wont hurt and we get little quicker all files downloaded so lower risk Viofo station mode turning off mid download. |
3 tasks
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.
Summary
Authored this to mitigate long inter-file dead time during sync — Wi-Fi saturated at N=1 download, but the worker spent N seconds per file on GPS extract + dashcam delete + mark_done after the download finished, during which the next file's bytes weren't flowing. Moving that tail to its own single-thread executor lets the next download start immediately.
During empirical testing on an A329 I realised the actual dominant culprit on my deployment was something else entirely: the SQLite state DB lives at
${RECORDINGS}/.viofosync.db, and with a NAS-backed recordings volume the per-file DB writes hit ~30 s lock-acquisition stalls (SQLitefcntl()over NFS waiting onbusy_timeout=30000while NFS write-back flushes the freshly-downloaded MP4). That's a separate architectural issue I'll look at next.But this pipelining change still feels like an improvement on its own — worth ~6 s per file on cleaner setups, and it's a no-op when the new
PIPELINE_POST_DOWNLOADsetting is off — so making the PR anyway.What's in
PIPELINE_POST_DOWNLOADsetting (default on). When on, the tail stage runs on a dedicatedThreadPoolExecutor(max_workers=1, name=viofo-tail)while the main worker picks up the next pending file.download_queue(download_started_at,download_finished_at,tail_submitted_at,tail_finished_at) for attributing cycle wall-clock. Idempotent ALTERs.cycle done: drained=N duration=Ts pipeline=<bool>.Test plan
pytest— full suite passes (147 + 4 new).viofo-tailthread when pipelining is on.PIPELINE_POST_DOWNLOAD=false, confirmed legacy sequential behaviour preserved.