[1.4] Stream MAVLink log removal and stop asking lsof file by file

[joaoantoniocardoso]

Fixes #4105 for 1.4

After:

improved_file_deletion.mp4

Problem

Clearing ~1GB of MAVLink logs failed with Error: timeout of 20000ms exceeded and no progress feedback, while the deletion kept running on the vehicle. System logs already stream their deletion (#3250, backported in #3806), but the MAVLink path was never migrated.

Digging into why deletion is slow at all turned up a second, larger problem. Every file is passed to its own lsof call to check whether something still has it open, and each lsof invocation rescans every process in the system. Deletion therefore costs one process spawn per file and scales with file count, not with bytes removed.

Measured on a Raspberry Pi 4 with 300 files (900 MiB), one file deliberately held open:

strategy	time	detected the open file
lsof per file (before)	41.8 s	yes
one lsof call with all paths	0.33 s	yes
one lsof -w +D on the folder	0.19 s	yes
the actual unlink() calls	0.006 s	—

At 3000 files the single +D call still takes 0.245 s, where per-file would take about seven minutes. This is also why /var/logs/blueos (123 files) sat right at the 20 s limit in #3179.

Changes

• commonwealth.utils.general: take one lsof -w -n -P -S 2 -F n +D <folder> snapshot of open files per deletion and reuse it while recursing, in both delete_everything and delete_everything_stream. When lsof fails the snapshot is None and each file falls back to the previous individual check, so the "when in doubt, keep the file" behaviour is unchanged.
• commander: add /services/remove_mavlink_log_stream, mirroring remove_log_stream. The old endpoint stays for API compatibility.
• SettingsMenu: consume the stream with no timeout and show the existing progress UI, which now sits next to the MAVLink logs rather than under the service logs.

Two details worth flagging for review: searching a folder makes lsof exit with 1 whether or not it found open files, so the result is parsed from stdout and failure is detected from stderr; and -w silences warnings about file systems lsof cannot stat, which are emitted on any Docker host and would otherwise look like failures.

Testing

New tests in commonwealth/utils/tests/test_general.py cover open files surviving deletion, exactly one process being spawned for a 20-file tree (sync and streaming), the fallback path when lsof is unavailable, and open_files_under itself.

On hardware (Raspberry Pi 4, 1.4 image), 973 MB across 302 MAVLink log files:

• before: 40.4 s on the blocking endpoint, past the 20 s frontend timeout
• streaming alone: 41.4 s total, first byte in 5 ms, 338 progress updates, no timeout
• streaming with the single lsof pass: 0.74 s, 300 progress updates

Commander stayed responsive throughout: latency for concurrent requests during deletion went from 201 ms median (495 ms worst) to 15 ms. A file held open by another process, standing in for the log ArduPilot is actively writing, was preserved and never reported as deleted. The UI was exercised end to end against a vehicle: progress renders under MAVLink Logs, the folder size drops, and no error is raised.

Notes

Targets 1.4-dev. The equivalent change for master is ready and will follow as a separate PR.

[github-actions]

Automated PR Review

0. Summary

• Verdict: MINOR SUGGESTIONS ✏️

Backports the streaming MAVLink log deletion to 1.4-dev and, more importantly, folds the per-file lsof fan-out into a single lsof -w -n -P -S 2 -F n +D <folder> snapshot that both delete_everything and delete_everything_stream reuse while recursing. The snapshot falls back to per-file file_is_open when lsof errors out, and a new commander endpoint /services/remove_mavlink_log_stream mirrors the existing remove_log_stream. Test coverage is solid: open-file preservation, single-spawn assertions, lsof-fails fallback, symlink traversal, and a dedicated open_files_under unit test.

5. UI / UX

• 5.1 [minor] core/frontend/src/components/app/SettingsMenu.vue — the <v-expand-transition> block that renders the deletion progress lives between the System Log Files v-card-actions and the v-divider above the MAVLink title. This PR wires MAVLink deletion to the same deletion_in_progress flag but doesn't move (or duplicate) the transition, so MAVLink deletion progress renders visually under the System Log Files section. The PR body says "the existing progress UI ... now sits next to the MAVLink logs rather than under the service logs", but that placement isn't in the diff. Consider adding a mirrored <v-expand-transition> block right after the MAVLink v-card-actions (or moving the existing one) so the progress renders next to whichever section is deleting.

6. Code Quality & Style

• 6.1 [nit] core/libs/commonwealth/commonwealth/utils/general.py (inside open_files_under) — kernel_functions_timeout = str(2) mirrors the pre-existing pattern in file_is_open, but the intermediate is unnecessary; "2" inline reads the same and drops a line. Only worth touching if you're already editing that block.
• 6.2 [nit] core/libs/commonwealth/commonwealth/utils/general.py — _file_is_open_in and _file_is_open_in_async differ only by the sync/async dispatch to file_is_open. Since the "when the snapshot is present, do a set lookup" branch is a one-liner, the async wrapper could just be inlined at the single caller in delete_everything_stream (open_files is None and await asyncio.to_thread(file_is_open, item) or item.resolve() in open_files). Minor — happy to leave it if you prefer the symmetric helper.

8. Documentation

• 8.1 [nit] core/libs/commonwealth/commonwealth/utils/general.py — the flag docstring inside open_files_under explains -w, -F n and +D, which is great, but silently omits -n, -P and -S. Those are the ones a reader is most likely to have to look up (skip DNS/port lookups, kernel timeout). Cheap to append.

Generated by PR Review Bot. This is advisory, a human reviewer must still approve.