diff --git a/shard_core/data_model/app_meta.py b/shard_core/data_model/app_meta.py index 44cbf5e..424e5bf 100644 --- a/shard_core/data_model/app_meta.py +++ b/shard_core/data_model/app_meta.py @@ -1,8 +1,9 @@ import datetime import json +import time from enum import Enum from pathlib import Path as FilePath -from typing import Optional, List, Dict, Union +from typing import Optional, List, Dict, NamedTuple, Union from pydantic import BaseModel, model_validator @@ -164,20 +165,59 @@ class InstalledAppWithMeta(InstalledApp): meta: AppMeta | None +class _LastAccessCacheEntry(NamedTuple): + value: Optional[datetime.datetime] + cached_at: float # time.monotonic() when this entry was stored + + +# The DB column installed_apps.last_access is the single source of truth for +# app last-access. This short-lived cache keeps the per-request debounce read +# off the DB on the forwardAuth hot path (loading app assets), which is fired +# on every request to an app — see issue #133 and the auth-cache rationale +# (issue #89). Entries are refreshed on every write, so the cached value always +# equals the last persisted last_access. Invalidated on on_apps_update so an +# uninstall/reinstall of the same name never serves a stale time. +_last_access_cache: Dict[str, _LastAccessCacheEntry] = {} + + +@signals.on_apps_update.connect +async def _invalidate_last_access_cache(_): + _last_access_cache.clear() + + +async def get_last_access(app_name: str) -> Optional[datetime.datetime]: + ttl = settings().apps.last_access.read_cache_ttl + cached = _last_access_cache.get(app_name) + if cached is not None and time.monotonic() - cached.cached_at < ttl: + return cached.value + + from shard_core.database.connection import db_conn + from shard_core.database import installed_apps as db_installed_apps + + async with db_conn() as conn: + row = await db_installed_apps.get_by_name(conn, app_name) + value = row["last_access"] if row else None + if value is not None and value.tzinfo is None: + value = value.replace(tzinfo=datetime.timezone.utc) + _last_access_cache[app_name] = _LastAccessCacheEntry(value, time.monotonic()) + return value + + @signals.on_request_to_app.connect async def update_last_access(app: InstalledApp): now = datetime.datetime.now(datetime.timezone.utc) max_update_frequency = datetime.timedelta( seconds=settings().apps.last_access.max_update_frequency ) - if app.last_access and now - app.last_access < max_update_frequency: + last_access = await get_last_access(app.name) + if last_access and now - last_access < max_update_frequency: return from shard_core.database.connection import db_conn from shard_core.database import installed_apps as db_installed_apps async with db_conn() as conn: await db_installed_apps.update_last_access(conn, app.name, now) - app.last_access = now + _last_access_cache[app.name] = _LastAccessCacheEntry(now, time.monotonic()) if __name__ == "__main__": diff --git a/shard_core/service/app_lifecycle.py b/shard_core/service/app_lifecycle.py index 79e690e..1701a39 100644 --- a/shard_core/service/app_lifecycle.py +++ b/shard_core/service/app_lifecycle.py @@ -1,7 +1,7 @@ import asyncio +import datetime import logging -import time -from typing import Dict, List +from typing import List from shard_core.database.connection import db_conn from shard_core.database import installed_apps as db_installed_apps @@ -20,21 +20,41 @@ log = logging.getLogger(__name__) -last_access_dict: Dict[str, float] = dict() background_tasks = set() # Pressure demotion never touches an app accessed within this window (seconds). RECENT_ACCESS_GRACE = 5 +# control_apps loads every app fresh from the DB each cycle, so idle decisions +# read InstalledApp.last_access directly — not through app_meta's request-path +# read cache, whose TTL must never delay a stop. +def _last_access_epoch(app: InstalledApp) -> float: + """Last-access as a POSIX timestamp; 0.0 (oldest) when never accessed.""" + if app.last_access is None: + return 0.0 + last = app.last_access + if last.tzinfo is None: + last = last.replace(tzinfo=datetime.timezone.utc) + return last.timestamp() + + +def _idle_seconds(app: InstalledApp) -> float: + """Seconds since the app was last accessed; inf when never accessed.""" + if app.last_access is None: + return float("inf") + now = datetime.datetime.now(datetime.timezone.utc).timestamp() + return now - _last_access_epoch(app) + + @signals.on_request_to_app.connect async def ensure_app_is_running(app: InstalledApp): if disk.current_disk_usage.disk_space_low: return app_meta = get_app_metadata(app.name) if await size_is_compatible(app_meta.minimum_portal_size): - global last_access_dict - last_access_dict[app.name] = time.time() + # last_access is recorded by update_last_access (app_meta.py), the other + # receiver of on_request_to_app; the DB column is the source of truth. # PAUSED wakes via unpause (ms to ~2s), everything else via the legacy # start path. Calling docker_unpause_app directly also dodges the # global 5s throttle on docker_start_app, which would silently drop @@ -87,7 +107,7 @@ async def _control_app_time(app: InstalledApp, pause_enabled: bool): await docker_start_app(app.name) return - idle = time.time() - last_access_dict.get(app.name, 0.0) + idle = _idle_seconds(app) t2 = ( app_meta.lifecycle.idle_for_stop or settings().apps.lifecycle.default_idle_for_stop @@ -127,7 +147,7 @@ async def _demote_lru(apps: List[InstalledApp]): app_meta = get_app_metadata(app.name) if app_meta.lifecycle.always_on: continue - if time.time() - last_access_dict.get(app.name, 0.0) <= RECENT_ACCESS_GRACE: + if _idle_seconds(app) <= RECENT_ACCESS_GRACE: continue if app.status == Status.RUNNING: running.append(app) @@ -135,7 +155,7 @@ async def _demote_lru(apps: List[InstalledApp]): paused.append(app) def lru(candidates: List[InstalledApp]) -> InstalledApp: - return min(candidates, key=lambda app: last_access_dict.get(app.name, 0.0)) + return min(candidates, key=_last_access_epoch) if running: victim = lru(running) diff --git a/shard_core/settings.py b/shard_core/settings.py index 10fb2eb..cf25ce4 100644 --- a/shard_core/settings.py +++ b/shard_core/settings.py @@ -54,6 +54,7 @@ class AppLifecycleSettings(BaseModel): class AppLastAccessSettings(BaseModel): max_update_frequency: int = 60 + read_cache_ttl: int = 5 # seconds the debounce read stays cached off the DB class AppUsageReportingSettings(BaseModel): diff --git a/tests/test_app_last_access_cache.py b/tests/test_app_last_access_cache.py new file mode 100644 index 0000000..6ef26c2 --- /dev/null +++ b/tests/test_app_last_access_cache.py @@ -0,0 +1,49 @@ +"""Unit tests for the last-access read cache (app_meta.get_last_access). + +The DB is mocked — these pin that the cache actually elides DB reads within its +TTL and re-reads once it expires, which is the whole point of the cache added in +#133 (keep the forwardAuth hot path off the DB). +""" + +from contextlib import asynccontextmanager +from datetime import datetime, timezone +from unittest.mock import AsyncMock, patch + +from shard_core.data_model import app_meta +from tests.conftest import settings_override + + +@asynccontextmanager +async def _fake_conn(): + yield None + + +async def test_get_last_access_serves_repeat_reads_from_cache(): + app_meta._last_access_cache.clear() + ts = datetime(2026, 1, 1, tzinfo=timezone.utc) + get_by_name = AsyncMock(return_value={"last_access": ts}) + with ( + settings_override({"apps": {"last_access": {"read_cache_ttl": 3600}}}), + patch("shard_core.database.connection.db_conn", _fake_conn), + patch("shard_core.database.installed_apps.get_by_name", get_by_name), + ): + first = await app_meta.get_last_access("foo") + second = await app_meta.get_last_access("foo") + + assert first == ts + assert second == ts + assert get_by_name.call_count == 1 # second read served from cache + + +async def test_get_last_access_rereads_after_ttl_expiry(): + app_meta._last_access_cache.clear() + get_by_name = AsyncMock(return_value={"last_access": None}) + with ( + settings_override({"apps": {"last_access": {"read_cache_ttl": 0}}}), + patch("shard_core.database.connection.db_conn", _fake_conn), + patch("shard_core.database.installed_apps.get_by_name", get_by_name), + ): + await app_meta.get_last_access("foo") + await app_meta.get_last_access("foo") + + assert get_by_name.call_count == 2 # ttl=0 never serves from cache diff --git a/tests/test_app_lifecycle.py b/tests/test_app_lifecycle.py index 45a7a92..4fb03a1 100644 --- a/tests/test_app_lifecycle.py +++ b/tests/test_app_lifecycle.py @@ -1,7 +1,11 @@ +from datetime import datetime, timedelta, timezone +from unittest.mock import AsyncMock, patch + import docker from fastapi import status -from shard_core.data_model.app_meta import InstalledApp, Status +from shard_core.data_model.app_meta import AppMeta, InstalledApp, Lifecycle, Status +from shard_core.service import app_lifecycle from tests.util import retry_async, wait_until_app_installed @@ -101,3 +105,61 @@ async def assert_app_running(): # todo: test_large_app_does_not_start # todo: test app with size comparison + + +# Idle-stop reads the DB last_access column (installed_apps.last_access), which +# is the single source of truth since #133 replaced the in-memory dict. The +# app object passed to _control_app_time is the one control_apps loads from the +# DB, so setting InstalledApp.last_access here pins the column-driven decision. +# tests/config.toml: default_idle_for_stop=12. + + +def _idle_meta() -> AppMeta: + return AppMeta( + v="1.3", + app_version="1.0.0", + name="test", + pretty_name="Test", + icon="icon.svg", + entrypoints=[], + paths={}, + lifecycle=Lifecycle(), + ) + + +async def test_idle_stop_uses_last_access_when_stale(): + app = InstalledApp( + name="a", + status=Status.RUNNING, + last_access=datetime.now(timezone.utc) - timedelta(seconds=20), + ) + with ( + patch.object(app_lifecycle, "docker_stop_app", new=AsyncMock()) as stop, + patch.object(app_lifecycle, "get_app_metadata", return_value=_idle_meta()), + ): + await app_lifecycle._control_app_time(app, pause_enabled=False) + stop.assert_awaited_once_with("a") + + +async def test_idle_stop_leaves_recently_accessed_app_running(): + app = InstalledApp( + name="a", + status=Status.RUNNING, + last_access=datetime.now(timezone.utc), + ) + with ( + patch.object(app_lifecycle, "docker_stop_app", new=AsyncMock()) as stop, + patch.object(app_lifecycle, "get_app_metadata", return_value=_idle_meta()), + ): + await app_lifecycle._control_app_time(app, pause_enabled=False) + stop.assert_not_awaited() + + +async def test_never_accessed_running_app_is_treated_as_idle(): + app = InstalledApp(name="a", status=Status.RUNNING, last_access=None) + with ( + patch.object(app_lifecycle, "docker_stop_app", new=AsyncMock()) as stop, + patch.object(app_lifecycle, "get_app_metadata", return_value=_idle_meta()), + ): + await app_lifecycle._control_app_time(app, pause_enabled=False) + stop.assert_awaited_once_with("a") diff --git a/tests/test_app_lifecycle_pause.py b/tests/test_app_lifecycle_pause.py index 23692fa..d01e1da 100644 --- a/tests/test_app_lifecycle_pause.py +++ b/tests/test_app_lifecycle_pause.py @@ -5,7 +5,7 @@ (that lives in the integration tests). """ -import time +from datetime import datetime, timedelta, timezone from unittest.mock import AsyncMock, patch import pytest @@ -47,16 +47,9 @@ def docker_mocks(): yield {"start": start, "stop": stop, "pause": pause, "unpause": unpause} -@pytest.fixture(autouse=True) -def clean_last_access(): - app_lifecycle.last_access_dict.clear() - yield - app_lifecycle.last_access_dict.clear() - - def _app(name: str, status: Status, idle: float) -> InstalledApp: - app_lifecycle.last_access_dict[name] = time.time() - idle - return InstalledApp(name=name, status=status) + last_access = datetime.now(timezone.utc) - timedelta(seconds=idle) + return InstalledApp(name=name, status=status, last_access=last_access) PAUSE_ON = {"apps": {"lifecycle": {"pause_enabled": True}}} @@ -277,3 +270,16 @@ async def test_demote_lru_demotes_exactly_one_app_per_cycle(docker_mocks): await app_lifecycle._demote_lru(apps) assert docker_mocks["pause"].await_count == 1 docker_mocks["pause"].assert_awaited_once_with("a") + + +async def test_demote_lru_treats_never_accessed_app_as_oldest(docker_mocks): + apps = [ + _app("recent", Status.RUNNING, idle=100), + InstalledApp(name="never", status=Status.RUNNING, last_access=None), + ] + with patch.object( + app_lifecycle, "get_app_metadata", return_value=_meta(Lifecycle()) + ): + await app_lifecycle._demote_lru(apps) + docker_mocks["pause"].assert_awaited_once_with("never") + docker_mocks["stop"].assert_not_awaited()