mirror of
https://github.com/Akkudoktor-EOS/EOS.git
synced 2026-08-06 08:56:12 +00:00
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 <cornim@users.noreply.github.com>
This commit is contained in:
co-authored by
Cornelius Mund
parent
1905682113
commit
b59012c1f7
@@ -620,8 +620,37 @@ class DatabaseRecordProtocolMixin(
|
|||||||
self._db_metadata = None
|
self._db_metadata = None
|
||||||
self._db_storage_initialized: bool = False
|
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
|
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:
|
if not self._db_storage_initialized and self.db_enabled:
|
||||||
# Metadata
|
# Metadata
|
||||||
existing_metadata = await self._db_load_metadata()
|
existing_metadata = await self._db_load_metadata()
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import tempfile
|
|||||||
import time
|
import time
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import AsyncIterator, Optional, Type
|
from typing import AsyncIterator, Optional, Type
|
||||||
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
import pytest_asyncio
|
import pytest_asyncio
|
||||||
@@ -160,6 +161,59 @@ class TestDataSequenceDatabaseProtocol:
|
|||||||
await _reset_sequence_state(sequence)
|
await _reset_sequence_state(sequence)
|
||||||
assert sequence.db_enabled is False
|
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):
|
async def test_insert_and_save_records(self, async_database_instance):
|
||||||
sequence = SampleDataSequence()
|
sequence = SampleDataSequence()
|
||||||
await _reset_sequence_state(sequence)
|
await _reset_sequence_state(sequence)
|
||||||
|
|||||||
Reference in New Issue
Block a user