From b59012c1f7f6ef80f7f0e6dd21467e77878bc214 Mon Sep 17 00:00:00 2001 From: Cornelius Mund <4417017+cornim@users.noreply.github.com> Date: Sat, 1 Aug 2026 12:46:00 +0200 Subject: [PATCH] fix(database): open backend so records actually persist and reload (#1209) * fix(database): open backend so records actually persist and reload Measurement and prediction records were never persisted to the configured database backend (e.g. LMDB). They only survived while the process was running; every restart lost the accumulated history. For LoadAkkudoktorAdjusted this silently zeroed the measurement-based adjustment (the adjusted load forecast collapsed onto the raw mean). Root cause: `db_enabled` is defined as `database.is_open`, and every database code path (initialization, load, save, insert) is guarded behind `db_enabled`. Nothing ever opened the backend first -- the only lazy open (via `_run_db`) was unreachable because those calls sit behind the same guard. The backend therefore stayed closed, `db_enabled` stayed False, and all writes fell through to the JSON file fallback, which only holds the current in-memory snapshot and is not reloaded into the record store on startup. Fix: explicitly open the configured backend in `_db_ensure_initialized` before the `db_enabled` gate, wrapped in try/except so a failure degrades gracefully to file storage. Verified via an A/B test (identical persisted config, push -> save -> restart): without the fix the backend reports enabled=false and data is lost on restart; with the fix the backend is enabled, records persist to LMDB, and reload correctly after restart. * fix(database): skip disabled providers and avoid open retry loop Address review feedback: - Skip opening the backend for disabled providers (None/"NoDB"), whose is_open is always False and would otherwise be reopened on every call. - Add a one-shot _db_open_attempted flag so a closed/failed backend is not retried (and re-logged) on every record operation. - Add tests covering that NoDB never calls open() and that an unavailable backend does not raise per operation and is opened at most once. * fix(database): drop file-storage-fallback wording from open failure log --------- Co-authored-by: Cornelius Mund --- src/akkudoktoreos/core/databaseabc.py | 29 ++++++++++++++ tests/test_dataabcsequencedb.py | 54 +++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/src/akkudoktoreos/core/databaseabc.py b/src/akkudoktoreos/core/databaseabc.py index 4397bee3..652f7ea3 100644 --- a/src/akkudoktoreos/core/databaseabc.py +++ b/src/akkudoktoreos/core/databaseabc.py @@ -620,8 +620,37 @@ class DatabaseRecordProtocolMixin( self._db_metadata = None self._db_storage_initialized: bool = False + # Whether an explicit backend open has already been attempted. + # One-shot per init so a closed/failed backend is not retried on + # every record operation. Re-armed when DB state is reset. + self._db_open_attempted: bool = False + self._db_initialized: bool = True + # Ensure the configured database backend is actually opened. + # `db_enabled` reflects `database.is_open`, and every DB code path is guarded + # behind `db_enabled`. Without an explicit open here the backend is never + # opened (lazy open via `_run_db` is unreachable), so records would only ever + # persist to the JSON file fallback and never reload into memory on startup. + # + # Skip disabled providers (None/"NoDB"): persistence is intentionally off and + # `NoDB.is_open` is always False, so opening it would run on every call. Only + # attempt the open once; on failure we log without retrying (and re-logging) + # on every subsequent record operation. + provider = self.config.database.provider + if ( + provider not in (None, "NoDB") + and not self.database.is_open + and not self._db_open_attempted + ): + self._db_open_attempted = True + try: + await self.database.open(namespace=self.db_namespace()) + except Exception: + logger.exception( + f"Could not open database backend for namespace '{self.db_namespace()}'." + ) + if not self._db_storage_initialized and self.db_enabled: # Metadata existing_metadata = await self._db_load_metadata() diff --git a/tests/test_dataabcsequencedb.py b/tests/test_dataabcsequencedb.py index 29250548..f0a271d8 100644 --- a/tests/test_dataabcsequencedb.py +++ b/tests/test_dataabcsequencedb.py @@ -11,6 +11,7 @@ import tempfile import time from pathlib import Path from typing import AsyncIterator, Optional, Type +from unittest.mock import AsyncMock import pytest import pytest_asyncio @@ -160,6 +161,59 @@ class TestDataSequenceDatabaseProtocol: await _reset_sequence_state(sequence) assert sequence.db_enabled is False + async def test_nodb_provider_does_not_call_open(self, config_eos, monkeypatch): + """Disabled providers (None/NoDB) must not repeatedly open the backend. + + `NoDB.is_open` is always False, so without the provider guard every + `_db_ensure_initialized()` (and thus every record operation) would call + `Database.open()`. + """ + config_eos.database.provider = None + sequence = SampleDataSequence() + + open_mock = AsyncMock() + monkeypatch.setattr(sequence.database, "open", open_mock) + + await _reset_sequence_state(sequence) + # Several operations that all funnel through _db_ensure_initialized(). + await sequence._db_ensure_initialized() + await sequence.db_insert_record( + SampleDataRecord(date_time=to_datetime("2024-01-01T00:00:00Z"), temperature=1.0) + ) + await sequence.db_save_records() + await sequence.db_load_records() + + assert sequence.db_enabled is False + open_mock.assert_not_called() + + async def test_failed_open_is_not_retried_per_operation( + self, config_eos, monkeypatch + ): + """An unavailable backend must be opened at most once, not per operation. + + The failure is swallowed (falls back to file storage) and the one-shot + `_db_open_attempted` flag prevents retry/re-log on every record op. + """ + config_eos.database.provider = "LMDB" + sequence = SampleDataSequence() + + open_mock = AsyncMock(side_effect=RuntimeError("backend unavailable")) + monkeypatch.setattr(sequence.database, "open", open_mock) + + # None of these must raise despite the failing backend. + await _reset_sequence_state(sequence) + await sequence._db_ensure_initialized() + await sequence.db_insert_record( + SampleDataRecord(date_time=to_datetime("2024-01-01T00:00:00Z"), temperature=1.0) + ) + await sequence.db_save_records() + await sequence.db_load_records() + + assert sequence.db_enabled is False + assert open_mock.call_count == 1 + + config_eos.database.provider = None + async def test_insert_and_save_records(self, async_database_instance): sequence = SampleDataSequence() await _reset_sequence_state(sequence)