From 9a3433a733558ab28cc65c8644ab36166d1a36c5 Mon Sep 17 00:00:00 2001 From: "alexei.dolgolyov" Date: Wed, 25 Mar 2026 13:16:35 +0300 Subject: [PATCH] fix: remove destructive DELETE+INSERT shutdown save that caused progressive data loss _save_all() in BaseSqliteStore did DELETE FROM table + INSERT all in-memory items on every shutdown. Since SQLite stores use write-through caching (every CRUD writes immediately), this was redundant. Worse, if in-memory state had fewer items than the DB, the DELETE wiped rows and only partial data was reinserted. - Make _save_all() a no-op (DB is always up to date via write-through) - Replace self._save() with self._save_item() in 6 seed/default creation methods - Remove _save_all_stores() shutdown hook (replaced with log-only message) --- server/src/wled_controller/main.py | 22 +++---------- .../storage/audio_template_store.py | 2 +- .../storage/base_sqlite_store.py | 32 +++---------------- .../color_strip_processing_template_store.py | 2 +- .../wled_controller/storage/gradient_store.py | 5 +-- .../storage/pattern_template_store.py | 2 +- .../storage/postprocessing_template_store.py | 2 +- .../wled_controller/storage/template_store.py | 2 +- 8 files changed, 17 insertions(+), 52 deletions(-) diff --git a/server/src/wled_controller/main.py b/server/src/wled_controller/main.py index 7e28934..b199e48 100644 --- a/server/src/wled_controller/main.py +++ b/server/src/wled_controller/main.py @@ -103,26 +103,12 @@ processor_manager = ProcessorManager( def _save_all_stores() -> None: - """Persist every store to disk. + """Shutdown hook — SQLite stores use write-through caching, so this is a no-op. - Called during graceful shutdown to ensure in-memory data survives - restarts even if no CRUD happened during the session. + Every create/update/delete already goes to the database immediately. + Kept for backward compatibility with server_ref.py which calls this. """ - all_stores = [ - device_store, template_store, pp_template_store, - picture_source_store, output_target_store, pattern_template_store, - color_strip_store, audio_source_store, audio_template_store, - value_source_store, automation_store, scene_preset_store, - sync_clock_store, cspt_store, gradient_store, weather_source_store, - ] - saved = 0 - for store in all_stores: - try: - store._save(force=True) - saved += 1 - except Exception as e: - logger.error(f"Failed to save {store._json_key} on shutdown: {e}") - logger.info(f"Shutdown save: persisted {saved}/{len(all_stores)} stores to disk") + logger.info("Shutdown: all stores already persisted (write-through cache)") @asynccontextmanager diff --git a/server/src/wled_controller/storage/audio_template_store.py b/server/src/wled_controller/storage/audio_template_store.py index 27bf7c3..ee54959 100644 --- a/server/src/wled_controller/storage/audio_template_store.py +++ b/server/src/wled_controller/storage/audio_template_store.py @@ -58,7 +58,7 @@ class AudioTemplateStore(BaseSqliteStore[AudioCaptureTemplate]): ) self._items[template_id] = template - self._save() + self._save_item(template_id, template) logger.info( f"Auto-created initial audio template: {template.name} " f"({template_id}, engine={best_engine})" diff --git a/server/src/wled_controller/storage/base_sqlite_store.py b/server/src/wled_controller/storage/base_sqlite_store.py index 0901fc9..7f22581 100644 --- a/server/src/wled_controller/storage/base_sqlite_store.py +++ b/server/src/wled_controller/storage/base_sqlite_store.py @@ -71,35 +71,13 @@ class BaseSqliteStore(Generic[T]): self._db.delete_row(self._table_name, item_id) def _save_all(self, *, force: bool = False) -> None: - """Persist all items to SQLite. + """No-op — SQLite stores use write-through caching. - Used during shutdown to ensure in-memory state is flushed. - When ``force`` is True, bypasses the frozen-writes check. + Every create/update calls ``_save_item()`` and every delete calls + ``_delete_item()``, so the database is always up to date. + A bulk DELETE + re-INSERT here would be destructive if in-memory + state diverged from the DB (e.g., partial load on startup). """ - from wled_controller.storage.database import _writes_frozen - if _writes_frozen and not force: - logger.warning(f"Save blocked (frozen after restore): {self._table_name}") - return - - items_to_write = [] - with self._lock: - for item_id, item in self._items.items(): - data = item.to_dict() - import json - items_to_write.append(( - item_id, - data.get("name", ""), - json.dumps(data, ensure_ascii=False), - )) - - if items_to_write: - # Use transaction for atomicity: clear + re-insert - with self._db.transaction() as conn: - conn.execute(f"DELETE FROM [{self._table_name}]") - conn.executemany( - f"INSERT INTO [{self._table_name}] (id, name, data) VALUES (?, ?, ?)", - items_to_write, - ) # -- Backward compat: _save() used by subclass create/update methods ----- diff --git a/server/src/wled_controller/storage/color_strip_processing_template_store.py b/server/src/wled_controller/storage/color_strip_processing_template_store.py index 2a0652d..f8f6e1b 100644 --- a/server/src/wled_controller/storage/color_strip_processing_template_store.py +++ b/server/src/wled_controller/storage/color_strip_processing_template_store.py @@ -55,7 +55,7 @@ class ColorStripProcessingTemplateStore(BaseSqliteStore[ColorStripProcessingTemp ) self._items[template_id] = template - self._save() + self._save_item(template_id, template) logger.info(f"Auto-created initial color strip processing template: {template.name} ({template_id})") def _validate_strip_filters(self, filters: List[FilterInstance]) -> None: diff --git a/server/src/wled_controller/storage/gradient_store.py b/server/src/wled_controller/storage/gradient_store.py index b87da82..1518f69 100644 --- a/server/src/wled_controller/storage/gradient_store.py +++ b/server/src/wled_controller/storage/gradient_store.py @@ -58,7 +58,7 @@ class GradientStore(BaseSqliteStore[Gradient]): now = datetime.now(timezone.utc) for name, tuples in _BUILTIN_DEFS.items(): gid = f"gr_builtin_{name}" - self._items[gid] = Gradient( + gradient = Gradient( id=gid, name=name.capitalize(), stops=_tuples_to_stops(tuples), @@ -67,7 +67,8 @@ class GradientStore(BaseSqliteStore[Gradient]): updated_at=now, description=f"Built-in {name} gradient", ) - self._save() + self._items[gid] = gradient + self._save_item(gid, gradient) logger.info(f"Seeded {len(_BUILTIN_DEFS)} built-in gradients") # Aliases diff --git a/server/src/wled_controller/storage/pattern_template_store.py b/server/src/wled_controller/storage/pattern_template_store.py index 9189fc3..610ba13 100644 --- a/server/src/wled_controller/storage/pattern_template_store.py +++ b/server/src/wled_controller/storage/pattern_template_store.py @@ -52,7 +52,7 @@ class PatternTemplateStore(BaseSqliteStore[PatternTemplate]): ) self._items[template_id] = template - self._save() + self._save_item(template_id, template) logger.info(f"Auto-created initial pattern template: {template.name} ({template_id})") def create_template( diff --git a/server/src/wled_controller/storage/postprocessing_template_store.py b/server/src/wled_controller/storage/postprocessing_template_store.py index 44a9071..4b5fab9 100644 --- a/server/src/wled_controller/storage/postprocessing_template_store.py +++ b/server/src/wled_controller/storage/postprocessing_template_store.py @@ -57,7 +57,7 @@ class PostprocessingTemplateStore(BaseSqliteStore[PostprocessingTemplate]): ) self._items[template_id] = template - self._save() + self._save_item(template_id, template) logger.info(f"Auto-created initial postprocessing template: {template.name} ({template_id})") def create_template( diff --git a/server/src/wled_controller/storage/template_store.py b/server/src/wled_controller/storage/template_store.py index 675d276..20391e3 100644 --- a/server/src/wled_controller/storage/template_store.py +++ b/server/src/wled_controller/storage/template_store.py @@ -59,7 +59,7 @@ class TemplateStore(BaseSqliteStore[CaptureTemplate]): ) self._items[template_id] = template - self._save() + self._save_item(template_id, template) logger.info(f"Auto-created initial template: {template.name} ({template_id}, engine={best_engine})") def create_template(