Improve periodic sync reliability and make it optional (#8647)

* Improve recordings sync reliability

* Cleanup

* Formatting

* Make logs consistent

* Make syncing optional

docs/docs/configuration/index.md

@@ -350,8 +350,8 @@ record:
   # Optional: Number of minutes to wait between cleanup runs (default: shown below)
   # This can be used to reduce the frequency of deleting recording segments from disk if you want to minimize i/o
   expire_interval: 60
-  # Optional: Sync recordings with disk on startup (default: shown below).
+  # Optional: Sync recordings with disk on startup and once a day (default: shown below).
-  sync_on_startup: False
+  sync_recordings: False
   # Optional: Retention settings for recording
   retain:
     # Optional: Number of days to retain recordings regardless of events (default: shown below)

docs/docs/configuration/record.md

@@ -87,11 +87,11 @@ The export page in the Frigate WebUI allows for exporting real time clips with a
 
 ## Syncing Recordings With Disk
 
-In some cases the recordings files may be deleted but Frigate will not know this has happened. Sync on startup can be enabled which will tell Frigate to check the file system and delete any db entries for files which don't exist.
+In some cases the recordings files may be deleted but Frigate will not know this has happened. Recordings sync can be enabled which will tell Frigate to check the file system and delete any db entries for files which don't exist.
 
 ```yaml
 record:
-  sync_on_startup: True
+  sync_recordings: True
 ```
 
 :::warning

frigate/config.py

@@ -259,8 +259,8 @@ class RecordExportConfig(FrigateBaseModel):
 
 class RecordConfig(FrigateBaseModel):
     enabled: bool = Field(default=False, title="Enable record on all cameras.")
-    sync_on_startup: bool = Field(
+    sync_recordings: bool = Field(
-        default=False, title="Sync recordings with disk on startup."
+        default=False, title="Sync recordings with disk on startup and once a day."
     )
     expire_interval: int = Field(
         default=60,

frigate/record/cleanup.py

@@ -176,9 +176,8 @@ class RecordingCleanup(threading.Thread):
 
     def run(self) -> None:
         # on startup sync recordings with disk if enabled
-        if self.config.record.sync_on_startup:
+        if self.config.record.sync_recordings:
             sync_recordings(limited=False)
-
             next_sync = get_tomorrow_at_time(3)
 
         # Expire tmp clips every minute, recordings and clean directories every hour.
@@ -189,7 +188,11 @@ class RecordingCleanup(threading.Thread):
 
             self.clean_tmp_clips()
 
-            if datetime.datetime.now().astimezone(datetime.timezone.utc) > next_sync:
+            if (
+                self.config.record.sync_recordings
+                and datetime.datetime.now().astimezone(datetime.timezone.utc)
+                > next_sync
+            ):
                 sync_recordings(limited=True)
                 next_sync = get_tomorrow_at_time(3)
 

frigate/record/util.py

@@ -31,13 +31,12 @@ def remove_empty_directories(directory: str) -> None:
 def sync_recordings(limited: bool) -> None:
     """Check the db for stale recordings entries that don't exist in the filesystem."""
 
-    def delete_db_entries_without_file(files_on_disk: list[str]) -> bool:
+    def delete_db_entries_without_file(check_timestamp: float) -> bool:
         """Delete db entries where file was deleted outside of frigate."""
 
         if limited:
             recordings = Recordings.select(Recordings.id, Recordings.path).where(
-                Recordings.start_time
+                Recordings.start_time >= check_timestamp
-                >= (datetime.datetime.now() - datetime.timedelta(hours=36)).timestamp()
             )
         else:
             # get all recordings in the db
@@ -50,9 +49,16 @@ def sync_recordings(limited: bool) -> None:
 
         for page in range(num_pages):
             for recording in recordings.paginate(page, page_size):
-                if recording.path not in files_on_disk:
+                if not os.path.exists(recording.path):
                     recordings_to_delete.add(recording.id)
 
+        if len(recordings_to_delete) == 0:
+            return True
+
+        logger.info(
+            f"Deleting {len(recordings_to_delete)} recording DB entries with missing files"
+        )
+
         # convert back to list of dictionaries for insertion
         recordings_to_delete = [
             {"id": recording_id} for recording_id in recordings_to_delete
@@ -64,10 +70,6 @@ def sync_recordings(limited: bool) -> None:
             )
             return False
 
-        logger.debug(
-            f"Deleting {len(recordings_to_delete)} recording DB entries with missing files"
-        )
-
         # create a temporary table for deletion
         RecordingsToDelete.create_table(temporary=True)
 
@@ -95,20 +97,37 @@ def sync_recordings(limited: bool) -> None:
             if not Recordings.select().where(Recordings.path == file).exists():
                 files_to_delete.append(file)
 
+        if len(files_to_delete) == 0:
+            return True
+
+        logger.info(
+            f"Deleting {len(files_to_delete)} recordings files with missing DB entries"
+        )
+
         if float(len(files_to_delete)) / max(1, len(files_on_disk)) > 0.5:
             logger.debug(
                 f"Deleting {(float(len(files_to_delete)) / len(files_on_disk)):2f}% of recordings DB entries, could be due to configuration error. Aborting..."
             )
-            return
+            return False
 
         for file in files_to_delete:
             os.unlink(file)
 
+        return True
+
     logger.debug("Start sync recordings.")
 
+    # start checking on the hour 36 hours ago
+    check_point = datetime.datetime.now().replace(
+        minute=0, second=0, microsecond=0
+    ).astimezone(datetime.timezone.utc) - datetime.timedelta(hours=36)
+    db_success = delete_db_entries_without_file(check_point.timestamp())
+
+    # only try to cleanup files if db cleanup was successful
+    if db_success:
         if limited:
             # get recording files from last 36 hours
-        hour_check = f"{RECORD_DIR}/{(datetime.datetime.now().astimezone(datetime.timezone.utc) - datetime.timedelta(hours=36)).strftime('%Y-%m-%d/%H')}"
+            hour_check = f"{RECORD_DIR}/{check_point.strftime('%Y-%m-%d/%H')}"
             files_on_disk = {
                 os.path.join(root, file)
                 for root, _, files in os.walk(RECORD_DIR)
@@ -123,10 +142,6 @@ def sync_recordings(limited: bool) -> None:
                 for file in files
             }
 
-    db_success = delete_db_entries_without_file(files_on_disk)
-
-    # only try to cleanup files if db cleanup was successful
-    if db_success:
         delete_files_without_db_entry(files_on_disk)
 
     logger.debug("End sync recordings.")