Metadata cache: hard LRU row cap to stop unbounded growth (7.6GB incident)
Investigation (not assumption): the cache's TTL eviction + junk cleanup ARE correct and DO run automatically every 6h (CacheEvictorJob, auto_fix=True). The real gap is there's NO SIZE CEILING — TTL-only eviction means 'how big can it get' = 'however much you fetch within the 30-day window', so heavy discovery/enrichment legitimately grew metadata_cache_entities to ~1.8M rows / 7.6 GB, bloating the main DB (a factor in the corruption incident). Fix — add a bounded LRU cap: - entities_to_evict_for_capacity(total, max_rows): pure decision fn (cap<=0 disables), unit-testable like core.db_integrity.prune_backups. - MetadataCache.evict_over_capacity(): deletes the least-recently-ACCESSED rows (uses the already-stored last_accessed_at; NULL = never-touched = evicted first) down to the ceiling. Default 250k rows, tunable. - Wired as Phase 5 of CacheEvictorJob — runs LAST, after TTL/junk/orphan/null cleanup, so it only trims a still-oversized HEALTHY cache. Verified safe to bound/wipe: audited every cache reader (get_entity/ get_entities_batch/get_search_results/get_entity_detail/browse) — all degrade to None/[]/empty on miss, treated as 'go fetch'. Nothing depends on a row existing, so eviction can't break callers. Tests: tests/metadata/test_cache_capacity_eviction.py (8) — pure-fn coverage + real temp-DB proof that it drops the LRU rows specifically (not arbitrary) and NULL-access rows go first. 18 adjacent cache tests still green; ruff clean. Follow-ups (separate phases, scoped): (2) move the cache to its own bounded metadata_cache.sqlite3 (no JOINs to library tables — confirmed clean to split; invalidate-and-rebuild rather than migrate the 7.6GB), (3) kill the raw_json + 22-extracted-column double storage.
This commit is contained in:
parent
9231cbd506
commit
bb2241498f
3 changed files with 260 additions and 0 deletions
|
|
@ -14,6 +14,26 @@ from typing import Optional, Dict, List, Tuple
|
|||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Hard ceiling on cached entity rows. TTL-only eviction has no upper bound, so
|
||||
# heavy discovery/enrichment within the TTL window let this table reach ~1.8M
|
||||
# rows / 7.6 GB (and helped bloat the main DB toward a corruption incident).
|
||||
# Beyond this cap we evict least-recently-ACCESSED rows (LRU) — the data to do
|
||||
# it (last_accessed_at) is already stored. Tunable via config; 0 disables.
|
||||
_DEFAULT_MAX_CACHE_ENTITIES = 250_000
|
||||
|
||||
|
||||
def entities_to_evict_for_capacity(total_rows: int, max_rows: int) -> int:
|
||||
"""Pure decision: how many LRU rows to drop to bring the cache to ``max_rows``.
|
||||
|
||||
Separated from SQL so it's unit-testable (mirrors core.db_integrity.
|
||||
prune_backups). ``max_rows <= 0`` means "no cap" -> never evict. Never
|
||||
returns negative.
|
||||
"""
|
||||
if max_rows <= 0:
|
||||
return 0
|
||||
return max(0, total_rows - max_rows)
|
||||
|
||||
|
||||
# Singleton
|
||||
_cache_instance = None
|
||||
_cache_lock = threading.Lock()
|
||||
|
|
@ -619,6 +639,43 @@ class MetadataCache:
|
|||
|
||||
return self._run_maintenance_write("Cache eviction", _operation)
|
||||
|
||||
def evict_over_capacity(self, max_rows: int = _DEFAULT_MAX_CACHE_ENTITIES) -> int:
|
||||
"""Enforce a hard row ceiling: if the entity cache exceeds ``max_rows``,
|
||||
delete the least-recently-ACCESSED rows down to the cap (LRU). This is
|
||||
the bound TTL-only eviction lacked. Returns the count evicted.
|
||||
|
||||
Runs AFTER evict_expired in the maintenance job, so TTL-expired and junk
|
||||
rows are already gone — this only trims a still-oversized healthy cache.
|
||||
"""
|
||||
def _operation(conn):
|
||||
cursor = conn.cursor()
|
||||
total = cursor.execute(
|
||||
"SELECT COUNT(*) FROM metadata_cache_entities"
|
||||
).fetchone()[0]
|
||||
to_evict = entities_to_evict_for_capacity(total, max_rows)
|
||||
if to_evict <= 0:
|
||||
return 0
|
||||
# Delete the oldest-accessed rows. NULL last_accessed_at sorts first
|
||||
# (evicted earliest) — those were never touched since insert.
|
||||
cursor.execute("""
|
||||
DELETE FROM metadata_cache_entities
|
||||
WHERE id IN (
|
||||
SELECT id FROM metadata_cache_entities
|
||||
ORDER BY last_accessed_at ASC
|
||||
LIMIT ?
|
||||
)
|
||||
""", (to_evict,))
|
||||
evicted = cursor.rowcount
|
||||
conn.commit()
|
||||
if evicted > 0:
|
||||
logger.info(
|
||||
"Cache over capacity (%d > %d) — evicted %d least-recently-used entities",
|
||||
total, max_rows, evicted,
|
||||
)
|
||||
return evicted
|
||||
|
||||
return self._run_maintenance_write("Cache capacity eviction", _operation)
|
||||
|
||||
def clean_junk_entities(self) -> int:
|
||||
"""Delete cached entities with empty/placeholder names."""
|
||||
def _operation(conn):
|
||||
|
|
|
|||
|
|
@ -89,4 +89,21 @@ class CacheEvictorJob(RepairJob):
|
|||
logger.error("MB null cleanup failed: %s", e, exc_info=True)
|
||||
result.errors += 1
|
||||
|
||||
if context.check_stop():
|
||||
return result
|
||||
|
||||
# Phase 5: Hard capacity cap (LRU). Runs LAST so TTL/junk/orphan/null
|
||||
# rows are already gone; this only trims a still-oversized HEALTHY cache
|
||||
# down to the row ceiling — the bound TTL-only eviction never had (the
|
||||
# cache previously reached ~1.8M rows / 7.6 GB).
|
||||
try:
|
||||
over = cache.evict_over_capacity()
|
||||
result.auto_fixed += over
|
||||
result.scanned += over
|
||||
if over > 0:
|
||||
logger.info("Phase 5 — capacity cap: evicted %d LRU entries", over)
|
||||
except Exception as e:
|
||||
logger.error("Capacity eviction failed: %s", e, exc_info=True)
|
||||
result.errors += 1
|
||||
|
||||
return result
|
||||
|
|
|
|||
186
tests/metadata/test_cache_capacity_eviction.py
Normal file
186
tests/metadata/test_cache_capacity_eviction.py
Normal file
|
|
@ -0,0 +1,186 @@
|
|||
"""Tests for the metadata-cache hard capacity cap (LRU eviction).
|
||||
|
||||
TTL-only eviction had no upper bound, so heavy in-window caching let
|
||||
metadata_cache_entities reach ~1.8M rows / 7.6 GB. evict_over_capacity adds an
|
||||
LRU row ceiling. We test the pure decision function directly and the SQL
|
||||
behavior against a real temp DB (proves it drops the LEAST-recently-accessed
|
||||
rows, not arbitrary ones).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sqlite3
|
||||
import sys
|
||||
import types
|
||||
|
||||
import pytest
|
||||
|
||||
# Minimal stubs so importing core.metadata.cache doesn't drag in spotipy/config.
|
||||
if "spotipy" not in sys.modules:
|
||||
spotipy = types.ModuleType("spotipy")
|
||||
spotipy.Spotify = object
|
||||
oauth2 = types.ModuleType("spotipy.oauth2")
|
||||
oauth2.SpotifyOAuth = object
|
||||
oauth2.SpotifyClientCredentials = object
|
||||
spotipy.oauth2 = oauth2
|
||||
sys.modules["spotipy"] = spotipy
|
||||
sys.modules["spotipy.oauth2"] = oauth2
|
||||
if "config.settings" not in sys.modules:
|
||||
config_pkg = types.ModuleType("config")
|
||||
settings_mod = types.ModuleType("config.settings")
|
||||
|
||||
class _DummyCM:
|
||||
def get(self, key, default=None):
|
||||
return default
|
||||
|
||||
def get_active_media_server(self):
|
||||
return "plex"
|
||||
|
||||
settings_mod.config_manager = _DummyCM()
|
||||
config_pkg.settings = settings_mod
|
||||
sys.modules["config"] = config_pkg
|
||||
sys.modules["config.settings"] = settings_mod
|
||||
|
||||
from core.metadata.cache import ( # noqa: E402
|
||||
MetadataCache,
|
||||
entities_to_evict_for_capacity,
|
||||
)
|
||||
|
||||
|
||||
# ── pure decision function ─────────────────────────────────────────────────
|
||||
|
||||
def test_evict_count_over_cap():
|
||||
assert entities_to_evict_for_capacity(1000, 250) == 750
|
||||
|
||||
|
||||
def test_evict_count_at_or_under_cap_is_zero():
|
||||
assert entities_to_evict_for_capacity(250, 250) == 0
|
||||
assert entities_to_evict_for_capacity(10, 250) == 0
|
||||
|
||||
|
||||
def test_cap_zero_or_negative_means_no_eviction():
|
||||
assert entities_to_evict_for_capacity(10_000, 0) == 0
|
||||
assert entities_to_evict_for_capacity(10_000, -1) == 0
|
||||
|
||||
|
||||
def test_never_negative():
|
||||
assert entities_to_evict_for_capacity(0, 100) == 0
|
||||
|
||||
|
||||
# ── evict_over_capacity SQL behavior (real temp DB) ────────────────────────
|
||||
|
||||
class _NonClosingConn:
|
||||
def __init__(self, real):
|
||||
self._real = real
|
||||
|
||||
def cursor(self):
|
||||
return self._real.cursor()
|
||||
|
||||
def commit(self):
|
||||
return self._real.commit()
|
||||
|
||||
def close(self):
|
||||
pass
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *a):
|
||||
pass
|
||||
|
||||
|
||||
class _TempCache(MetadataCache):
|
||||
"""MetadataCache whose _get_db returns a shim over a shared in-memory DB
|
||||
holding just the entities table — enough to exercise evict_over_capacity."""
|
||||
|
||||
def __init__(self):
|
||||
self._conn = sqlite3.connect(":memory:")
|
||||
self._conn.row_factory = sqlite3.Row
|
||||
self._conn.execute("""
|
||||
CREATE TABLE metadata_cache_entities (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
source TEXT, entity_type TEXT, entity_id TEXT,
|
||||
name TEXT, raw_json TEXT,
|
||||
last_accessed_at TIMESTAMP,
|
||||
ttl_days INTEGER DEFAULT 30
|
||||
)
|
||||
""")
|
||||
self._conn.commit()
|
||||
|
||||
def _get_db(self):
|
||||
outer = self
|
||||
|
||||
class _DB:
|
||||
def _get_connection(self_inner):
|
||||
return _NonClosingConn(outer._conn)
|
||||
return _DB()
|
||||
|
||||
# evict_over_capacity uses self._run_maintenance_write -> _get_db; the base
|
||||
# _run_maintenance_write just calls the operation. Add a tiny passthrough
|
||||
# if the base needs it (it does in the real class).
|
||||
def _run_maintenance_write(self, label, operation, default=0):
|
||||
try:
|
||||
return operation(self._get_db()._get_connection())
|
||||
except Exception:
|
||||
return default
|
||||
|
||||
|
||||
def _add_rows(cache, specs):
|
||||
"""specs: list of (entity_id, last_accessed_at). Inserts rows."""
|
||||
cur = cache._conn.cursor()
|
||||
for eid, ts in specs:
|
||||
cur.execute(
|
||||
"INSERT INTO metadata_cache_entities (source, entity_type, entity_id, name, raw_json, last_accessed_at) "
|
||||
"VALUES ('spotify','artist',?,?, '{}', ?)",
|
||||
(eid, eid, ts),
|
||||
)
|
||||
cache._conn.commit()
|
||||
|
||||
|
||||
def _ids(cache):
|
||||
return [r["entity_id"] for r in cache._conn.execute(
|
||||
"SELECT entity_id FROM metadata_cache_entities ORDER BY entity_id"
|
||||
).fetchall()]
|
||||
|
||||
|
||||
def test_evict_over_capacity_drops_least_recently_accessed():
|
||||
cache = _TempCache()
|
||||
# 5 rows, distinct access times. Cap at 3 -> evict the 2 oldest-accessed.
|
||||
_add_rows(cache, [
|
||||
("a", "2026-05-01T00:00:00"), # oldest -> evicted
|
||||
("b", "2026-05-02T00:00:00"), # oldest -> evicted
|
||||
("c", "2026-05-03T00:00:00"),
|
||||
("d", "2026-05-04T00:00:00"),
|
||||
("e", "2026-05-05T00:00:00"), # newest -> kept
|
||||
])
|
||||
evicted = cache.evict_over_capacity(max_rows=3)
|
||||
assert evicted == 2
|
||||
assert _ids(cache) == ["c", "d", "e"] # a, b gone (LRU)
|
||||
|
||||
|
||||
def test_evict_over_capacity_noop_under_cap():
|
||||
cache = _TempCache()
|
||||
_add_rows(cache, [("a", "2026-05-01T00:00:00"), ("b", "2026-05-02T00:00:00")])
|
||||
assert cache.evict_over_capacity(max_rows=10) == 0
|
||||
assert _ids(cache) == ["a", "b"]
|
||||
|
||||
|
||||
def test_evict_over_capacity_disabled_with_zero_cap():
|
||||
cache = _TempCache()
|
||||
_add_rows(cache, [(str(i), f"2026-05-0{i}T00:00:00") for i in range(1, 6)])
|
||||
assert cache.evict_over_capacity(max_rows=0) == 0
|
||||
assert len(_ids(cache)) == 5
|
||||
|
||||
|
||||
def test_null_access_times_evicted_first():
|
||||
"""Rows never accessed since insert (NULL last_accessed_at) are the
|
||||
coldest — they should go before any touched row."""
|
||||
cache = _TempCache()
|
||||
_add_rows(cache, [
|
||||
("never1", None),
|
||||
("never2", None),
|
||||
("touched", "2026-05-01T00:00:00"),
|
||||
])
|
||||
evicted = cache.evict_over_capacity(max_rows=1)
|
||||
assert evicted == 2
|
||||
assert _ids(cache) == ["touched"]
|
||||
Loading…
Reference in a new issue