fix: prevent silent reject-ID data loss on partial migration rename
Two fixes found during post-refactor audit: 1. upload_tracker: remove COUNT(*) guard from _maybe_migrate. The guard blocked re-migration when a previous run successfully committed both JSON files but a PermissionError on the second rename() left it on disk. On the next startup COUNT > 0 → early return → rejected IDs permanently unimported. INSERT OR IGNORE is idempotent so re-running migration is always safe; guard not needed. Also wrap each rename() in its own try/except so a failure on one file is logged and does not propagate uncaught. 2. reconcile: break early when new_count > target is detected in the poll loop. Previously the loop ran all four delay intervals (1+2+4+8s) before the post-loop > target branch fired, wasting up to 15 seconds when a concurrent external upload was visible on the first poll.
This commit is contained in:
+4
-1
@@ -45,8 +45,11 @@ def reconcile_frigate_mappings(
|
||||
)
|
||||
return
|
||||
current_files = set(fresh)
|
||||
if len(current_files - known_files_before) == target:
|
||||
new_count = len(current_files - known_files_before)
|
||||
if new_count == target:
|
||||
break
|
||||
if new_count > target:
|
||||
break # external upload already visible — no point polling further
|
||||
|
||||
new_files = current_files - known_files_before
|
||||
|
||||
|
||||
@@ -105,10 +105,10 @@ def _maybe_migrate(cache_dir: str, conn: sqlite3.Connection) -> None:
|
||||
if not upload_json.exists() and not reject_json.exists():
|
||||
return
|
||||
|
||||
# Check if tables are already populated
|
||||
row = conn.execute("SELECT COUNT(*) FROM tracked_assets").fetchone()
|
||||
if row[0] > 0:
|
||||
return # already migrated
|
||||
# No row-count guard here: INSERT OR IGNORE makes migration idempotent, so it
|
||||
# is safe to re-run if a previous attempt renamed one file but not the other
|
||||
# (e.g. a PermissionError on the second rename would have left the first file's
|
||||
# data committed but the second file un-renamed and un-migrated).
|
||||
|
||||
logger.info("Migrating JSON tracker files to SQLite in %s", cache_dir)
|
||||
|
||||
@@ -122,11 +122,14 @@ def _maybe_migrate(cache_dir: str, conn: sqlite3.Connection) -> None:
|
||||
logger.warning("JSON migration failed, will retry next run: %s", exc)
|
||||
return
|
||||
|
||||
# Rename only after successful commit so a failed run retries cleanly next start.
|
||||
if upload_json.exists():
|
||||
upload_json.rename(upload_json.with_suffix(".json.bak"))
|
||||
if reject_json.exists():
|
||||
reject_json.rename(reject_json.with_suffix(".json.bak"))
|
||||
# Rename each file independently so a failure on one does not prevent the
|
||||
# other from being marked complete on this run.
|
||||
for json_path in (upload_json, reject_json):
|
||||
if json_path.exists():
|
||||
try:
|
||||
json_path.rename(json_path.with_suffix(".json.bak"))
|
||||
except OSError as exc:
|
||||
logger.warning("Could not rename %s after migration: %s", json_path, exc)
|
||||
|
||||
logger.info("JSON → SQLite migration complete")
|
||||
|
||||
|
||||
Reference in New Issue
Block a user