From 2da99c23087581deb30a1a2be123ecca0b3975fe Mon Sep 17 00:00:00 2001 From: Nicolas Mowen Date: Sat, 18 Nov 2023 05:06:00 -0700 Subject: [PATCH] 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 | 4 +- docs/docs/configuration/record.md | 4 +- frigate/config.py | 4 +- frigate/record/cleanup.py | 11 +++-- frigate/record/util.py | 69 +++++++++++++++++++------------ 5 files changed, 55 insertions(+), 37 deletions(-) diff --git a/docs/docs/configuration/index.md b/docs/docs/configuration/index.md index c3496f9ea..b0c964507 100644 --- a/docs/docs/configuration/index.md +++ b/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). - sync_on_startup: False + # Optional: Sync recordings with disk on startup and once a day (default: shown below). + sync_recordings: False # Optional: Retention settings for recording retain: # Optional: Number of days to retain recordings regardless of events (default: shown below) diff --git a/docs/docs/configuration/record.md b/docs/docs/configuration/record.md index a4739fa3b..5a505c6d1 100644 --- a/docs/docs/configuration/record.md +++ b/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 diff --git a/frigate/config.py b/frigate/config.py index a7ceaeecb..4c0db2441 100644 --- a/frigate/config.py +++ b/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( - default=False, title="Sync recordings with disk on startup." + sync_recordings: bool = Field( + default=False, title="Sync recordings with disk on startup and once a day." ) expire_interval: int = Field( default=60, diff --git a/frigate/record/cleanup.py b/frigate/record/cleanup.py index 7b3bc7e8d..c7aa0e167 100644 --- a/frigate/record/cleanup.py +++ b/frigate/record/cleanup.py @@ -176,10 +176,9 @@ 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) + next_sync = get_tomorrow_at_time(3) # Expire tmp clips every minute, recordings and clean directories every hour. for counter in itertools.cycle(range(self.config.record.expire_interval)): @@ -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) diff --git a/frigate/record/util.py b/frigate/record/util.py index 19b26a365..7388c04df 100644 --- a/frigate/record/util.py +++ b/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 - >= (datetime.datetime.now() - datetime.timedelta(hours=36)).timestamp() + Recordings.start_time >= check_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,38 +97,51 @@ 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.") - 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')}" - files_on_disk = { - os.path.join(root, file) - for root, _, files in os.walk(RECORD_DIR) - for file in files - if root > hour_check - } - else: - # get all recordings files on disk and put them in a set - files_on_disk = { - os.path.join(root, file) - for root, _, files in os.walk(RECORD_DIR) - for file in files - } - - db_success = delete_db_entries_without_file(files_on_disk) + # 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}/{check_point.strftime('%Y-%m-%d/%H')}" + files_on_disk = { + os.path.join(root, file) + for root, _, files in os.walk(RECORD_DIR) + for file in files + if root > hour_check + } + else: + # get all recordings files on disk and put them in a set + files_on_disk = { + os.path.join(root, file) + for root, _, files in os.walk(RECORD_DIR) + for file in files + } + delete_files_without_db_entry(files_on_disk) logger.debug("End sync recordings.")