Просмотр исходного кода

fix(camera): sweep orphaned timelapse session directories on startup

_active_sessions is in-memory only, so a process restart mid-print
loses track of any active layer-timelapse session without ever calling
cancel_session()/cleanup() - the frames directory (and, if stitching
had already produced output before the restart, a stray
timelapse_<session_id>.mp4) are then orphaned on disk permanently, with
no equivalent to the ffmpeg orphan janitor to reap them.

Confirmed live: 38MB of exactly this leftover on the OrangePi after
several restarts during this week's testing, including two corrupt
48-byte .mp4s from stitches that got interrupted mid-write.

Adds cleanup_orphaned_timelapse_sessions(), run once at startup: for
each printer_id under timelapse_frames/, remove any frame directory or
stitched-output file that doesn't match that printer's current active
session (if any) and is older than a defensive margin (5 min default).
A restart-recovered print never gets a new timelapse session either
(#1353's _maybe_start_layer_timelapse only fires on fresh PRINT_START
events), so nothing orphaned here can ever be resumed - safe to always
remove once it's old enough not to be a startup race.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Carl 1 месяц назад
Родитель
Сommit
24863203fb

+ 12 - 0
backend/app/main.py

@@ -6844,6 +6844,18 @@ async def lifespan(app: FastAPI):
     # Start camera stream orphan cleanup
     # Start camera stream orphan cleanup
     start_camera_cleanup()
     start_camera_cleanup()
 
 
+    # One-shot sweep for timelapse session directories orphaned by a crash
+    # or restart that happened mid-print (in-memory session tracking can't
+    # survive that, and nothing else reaps the leftover frames/output file)
+    try:
+        from backend.app.services.layer_timelapse import cleanup_orphaned_timelapse_sessions
+
+        removed = cleanup_orphaned_timelapse_sessions()
+        if removed:
+            logging.getLogger(__name__).info("Removed %d orphaned timelapse session artifact(s)", removed)
+    except Exception as e:
+        logging.getLogger(__name__).warning("Orphaned timelapse session cleanup failed: %s", e)
+
     # Start expected-print TTL eviction (prevents memory leak when prints are
     # Start expected-print TTL eviction (prevents memory leak when prints are
     # registered but on_print_start never fires)
     # registered but on_print_start never fires)
     start_expected_prints_cleanup()
     start_expected_prints_cleanup()

+ 64 - 0
backend/app/services/layer_timelapse.py

@@ -6,6 +6,7 @@ Captures a frame on each layer change and stitches them into a video on print co
 import asyncio
 import asyncio
 import logging
 import logging
 import shutil
 import shutil
+import time
 from dataclasses import dataclass, field
 from dataclasses import dataclass, field
 from datetime import datetime
 from datetime import datetime
 from pathlib import Path
 from pathlib import Path
@@ -303,3 +304,66 @@ def cancel_session(printer_id: int):
 def get_active_sessions() -> dict[int, TimelapseSession]:
 def get_active_sessions() -> dict[int, TimelapseSession]:
     """Get all active timelapse sessions."""
     """Get all active timelapse sessions."""
     return _active_sessions.copy()
     return _active_sessions.copy()
+
+
+def cleanup_orphaned_timelapse_sessions(min_age_seconds: float = 300) -> int:
+    """Remove timelapse_frames/<printer_id>/* left behind by a crash or
+    restart that happened while a session was active.
+
+    _active_sessions is in-memory only, so a process restart loses track of
+    any in-flight session without ever calling cancel_session()/cleanup() -
+    the frames directory (and, if stitching had already produced output
+    before the restart, a stray `timelapse_<session_id>.mp4`) are then
+    orphaned on disk with nothing else to reap them (unlike the ffmpeg
+    orphan janitor in routes/camera.py, there was no equivalent here).
+
+    Safe to call once at startup: normal operation always cleans up via
+    on_print_complete/cancel_session, so anything found here predates this
+    process - and a restart-recovered print doesn't get a new timelapse
+    session either (`_maybe_start_layer_timelapse` is only wired into fresh
+    PRINT_START events, see #1353), so an orphaned directory can never be
+    resumed. `min_age_seconds` is just a defensive margin against reordering
+    if this is ever also called mid-run.
+
+    Returns the number of orphaned directories/files removed.
+    """
+    base_dir = settings.base_dir / "timelapse_frames"
+    if not base_dir.exists():
+        return 0
+
+    now = time.time()
+    removed = 0
+    for printer_dir in base_dir.iterdir():
+        if not printer_dir.is_dir():
+            continue
+        try:
+            printer_id = int(printer_dir.name)
+        except ValueError:
+            continue
+
+        active_session = _active_sessions.get(printer_id)
+        active_session_id = active_session.session_id if active_session else None
+
+        for entry in printer_dir.iterdir():
+            # Frame dirs are named "<session_id>/"; stitched-but-not-yet-
+            # attached output files are "timelapse_<session_id>.mp4" (see
+            # on_print_complete's output_path).
+            entry_session_id = entry.name.removeprefix("timelapse_").removesuffix(".mp4") if entry.is_file() else entry.name
+            if entry_session_id == active_session_id:
+                continue
+            try:
+                if now - entry.stat().st_mtime < min_age_seconds:
+                    continue
+            except OSError:
+                continue
+            try:
+                if entry.is_dir():
+                    shutil.rmtree(entry, ignore_errors=True)
+                else:
+                    entry.unlink(missing_ok=True)
+                removed += 1
+                logger.info("Removed orphaned timelapse artifact: %s", entry)
+            except OSError as e:
+                logger.warning("Failed to remove orphaned timelapse artifact %s: %s", entry, e)
+
+    return removed

+ 115 - 0
backend/tests/unit/services/test_layer_timelapse.py

@@ -4,6 +4,7 @@ Tests for the layer timelapse service.
 These tests cover session management and pure logic functions.
 These tests cover session management and pure logic functions.
 """
 """
 
 
+import time
 from datetime import datetime
 from datetime import datetime
 from pathlib import Path
 from pathlib import Path
 from unittest.mock import AsyncMock, MagicMock, patch
 from unittest.mock import AsyncMock, MagicMock, patch
@@ -318,3 +319,117 @@ class TestGetActiveSessions:
                 assert 1 in _active_sessions
                 assert 1 in _active_sessions
 
 
                 cancel_session(1)
                 cancel_session(1)
+
+
+class TestCleanupOrphanedTimelapseSessions:
+    """_active_sessions is in-memory only, so a process restart mid-print
+    loses track of an active session without ever cleaning up its frames
+    directory (or a stitched-but-not-attached output .mp4). Confirmed live:
+    38MB of exactly this leftover on Carl's OrangePi after several restarts
+    during testing. cleanup_orphaned_timelapse_sessions() sweeps for it."""
+
+    def _touch_old(self, path, age_seconds=600):
+        import os
+
+        path.touch()
+        old = time.time() - age_seconds
+        os.utime(path, (old, old))
+
+    def _mkdir_old(self, path, age_seconds=600):
+        import os
+
+        path.mkdir(parents=True)
+        old = time.time() - age_seconds
+        os.utime(path, (old, old))
+
+    def test_removes_orphaned_frame_dir_and_stray_output(self, tmp_path):
+        from backend.app.services.layer_timelapse import (
+            _active_sessions,
+            cleanup_orphaned_timelapse_sessions,
+        )
+
+        _active_sessions.clear()
+        printer_dir = tmp_path / "timelapse_frames" / "1"
+        self._mkdir_old(printer_dir / "20260101_000000")
+        self._touch_old(printer_dir / "timelapse_20260101_000000.mp4")
+
+        with patch("backend.app.services.layer_timelapse.settings") as mock_settings:
+            mock_settings.base_dir = tmp_path
+            removed = cleanup_orphaned_timelapse_sessions(min_age_seconds=300)
+
+        assert removed == 2
+        assert not (printer_dir / "20260101_000000").exists()
+        assert not (printer_dir / "timelapse_20260101_000000.mp4").exists()
+
+    def test_spares_the_currently_active_session(self, tmp_path):
+        from backend.app.services.layer_timelapse import (
+            TimelapseSession,
+            _active_sessions,
+            cleanup_orphaned_timelapse_sessions,
+        )
+
+        _active_sessions.clear()
+        printer_dir = tmp_path / "timelapse_frames" / "1"
+
+        with patch("backend.app.services.layer_timelapse.settings") as mock_settings:
+            mock_settings.base_dir = tmp_path
+            session = TimelapseSession(1, 100, "/dev/video1", "usb")
+            _active_sessions[1] = session
+            import os
+
+            old = time.time() - 600
+            os.utime(session.frames_dir, (old, old))
+
+            removed = cleanup_orphaned_timelapse_sessions(min_age_seconds=300)
+
+        assert removed == 0
+        assert session.frames_dir.exists()
+        _active_sessions.clear()
+
+    def test_spares_recently_modified_entries(self, tmp_path):
+        """Defensive margin: something modified within min_age_seconds is
+        left alone even if it doesn't match an active session, in case this
+        is ever invoked while a session is mid-creation."""
+        from backend.app.services.layer_timelapse import (
+            _active_sessions,
+            cleanup_orphaned_timelapse_sessions,
+        )
+
+        _active_sessions.clear()
+        printer_dir = tmp_path / "timelapse_frames" / "1"
+        printer_dir.mkdir(parents=True)
+        (printer_dir / "20260101_000000").mkdir()
+
+        with patch("backend.app.services.layer_timelapse.settings") as mock_settings:
+            mock_settings.base_dir = tmp_path
+            removed = cleanup_orphaned_timelapse_sessions(min_age_seconds=300)
+
+        assert removed == 0
+        assert (printer_dir / "20260101_000000").exists()
+
+    def test_no_base_dir_is_a_no_op(self, tmp_path):
+        from backend.app.services.layer_timelapse import cleanup_orphaned_timelapse_sessions
+
+        with patch("backend.app.services.layer_timelapse.settings") as mock_settings:
+            mock_settings.base_dir = tmp_path / "does-not-exist"
+            removed = cleanup_orphaned_timelapse_sessions()
+
+        assert removed == 0
+
+    def test_ignores_non_numeric_printer_dirs(self, tmp_path):
+        """Defensive: unrelated directories under timelapse_frames/ (there
+        shouldn't be any, but printer_id is parsed from the dir name) must
+        not raise."""
+        from backend.app.services.layer_timelapse import (
+            _active_sessions,
+            cleanup_orphaned_timelapse_sessions,
+        )
+
+        _active_sessions.clear()
+        (tmp_path / "timelapse_frames" / "not-a-printer-id").mkdir(parents=True)
+
+        with patch("backend.app.services.layer_timelapse.settings") as mock_settings:
+            mock_settings.base_dir = tmp_path
+            removed = cleanup_orphaned_timelapse_sessions()
+
+        assert removed == 0