diff --git a/docs/LOW_MEMORY_BOARDS.md b/docs/LOW_MEMORY_BOARDS.md new file mode 100644 index 00000000..186e8a4a --- /dev/null +++ b/docs/LOW_MEMORY_BOARDS.md @@ -0,0 +1,115 @@ +# Running on Low-Memory Boards + +Applies to the Pi Zero 2 W (512 MB), Pi 3 / 3B+ (1 GB), and the 1 GB Pi 4. +If your board has 2 GB or more you can skip this document. + +## The failure this prevents + +The display process is the largest thing on the board. On a 1 GB Pi 3B+ with +around 20 plugins enabled it settles near **600 MB of 905 MB usable**, leaving +under 200 MB of headroom for everything else. + +When that headroom runs out, the board does not crash cleanly. `fork()` starts +failing, and because a new process is needed to do almost anything, the +symptoms look nothing like "out of memory": + +| What you see | Why | +|---|---| +| SSH accepts the connection then closes it instantly, before any banner | `sshd` forks a session per connection; the fork fails | +| The web UI still responds quickly | Already running, serves from existing threads, forks nothing | +| Ping is perfect, 0% loss | Handled entirely in the kernel | +| The panel is dark | The display process was killed and cannot be respawned | +| The clock is wrong after the next boot | `fake-hwclock`'s periodic save is a scheduled job, and it cannot fork either | + +The board looks healthy from the outside and cannot be logged into. Only a +power cycle clears it. If you are here because SSH stopped working, also see +[SSH_UNAVAILABLE_AFTER_INSTALL.md](SSH_UNAVAILABLE_AFTER_INSTALL.md), which +covers the more common cause (AP mode). + +## Check your headroom + +```bash +free -m +ps -eo rss,comm --sort=-rss | head -5 +``` + +If `MemAvailable` is under ~150 MB while the display is running, you are close +to the edge. To watch it over time: + +```bash +watch -n 30 'free -m | head -2' +``` + +Available memory that falls steadily rather than holding flat means you will +reach the wall; it is a question of when. + +## What to do + +**1. Enable the memory cgroup controller.** Without it, the `MemoryMax=85%` in +`systemd/ledmatrix.service` is accepted by systemd and silently ignored, so the +service has no ceiling and a runaway takes the whole board down instead of just +restarting. Raspberry Pi firmware disables this controller by default. + +`first_time_install.sh` does this for you. To check it took effect: + +```bash +grep memory /sys/fs/cgroup/cgroup.controllers +``` + +If that prints nothing, add `cgroup_enable=memory cgroup_memory=1` to the +kernel command line and reboot. Edit whichever file your image uses — +`/boot/firmware/cmdline.txt` on current Raspberry Pi OS, `/boot/cmdline.txt` on +older layouts (the installer checks the first and falls back to the second). +Everything must stay on a single line. + +This changes the failure mode from "the board becomes unreachable" to "the +display service restarts". It is a safety net, not a fix. + +**2. Run fewer plugins.** This is the actual remedy. Every enabled plugin costs +memory permanently — its module, its parsed config, and its cached API +responses. On a 512 MB or 1 GB board, keep the enabled set small and prefer +plugins that poll infrequently. + +**3. Lower the cache ceiling.** The in-memory cache is sized from total RAM +(150 entries at 1 GB and below, up to 1500 at 8 GB). To go lower still: + +```ini +# /etc/systemd/system/ledmatrix.service.d/override.conf +[Service] +Environment=LEDMATRIX_CACHE_MAX_ENTRIES=75 +``` + +Writing the file does not change the running service. Reload systemd and +restart it: + +```bash +sudo systemctl daemon-reload +sudo systemctl restart ledmatrix +``` + +Fewer entries means more API calls, so lower this only while you are actually +short of memory. + +**4. Consider `MemoryHigh`.** `MemoryMax` kills and restarts. `MemoryHigh` +throttles and reclaims instead, which is gentler — but on a board where the +process genuinely wants more than the limit, sustained reclaim can stall the +render loop and show as visible stutter on the panel. Add it only if you prefer +degraded output to a restart: + +```ini +[Service] +MemoryHigh=70% +``` + +## Keep your logs + +These images default to volatile journald storage, so every reboot destroys the +logs — including the ones explaining why the board rebooted. `first_time_install.sh` +enables persistent storage capped at 64 MB. To confirm: + +```bash +journalctl --list-boots +``` + +More than one boot listed means logs are surviving reboots. If only one is +listed, journald is still writing to `/run` (tmpfs). diff --git a/docs/README.md b/docs/README.md index acb2d944..63128da6 100644 --- a/docs/README.md +++ b/docs/README.md @@ -14,6 +14,7 @@ the one-shot installer. The pages here go deeper. 5. [TROUBLESHOOTING.md](TROUBLESHOOTING.md) — common issues and fixes 6. [SSH_UNAVAILABLE_AFTER_INSTALL.md](SSH_UNAVAILABLE_AFTER_INSTALL.md) — recovering SSH after install 7. [CONFIG_DEBUGGING.md](CONFIG_DEBUGGING.md) — diagnosing config problems +8. [LOW_MEMORY_BOARDS.md](LOW_MEMORY_BOARDS.md) — Pi Zero 2 W / 3B+ / 1GB Pi 4 memory limits ## I want to write a plugin diff --git a/docs/SSH_UNAVAILABLE_AFTER_INSTALL.md b/docs/SSH_UNAVAILABLE_AFTER_INSTALL.md index d9830165..bc065eb1 100644 --- a/docs/SSH_UNAVAILABLE_AFTER_INSTALL.md +++ b/docs/SSH_UNAVAILABLE_AFTER_INSTALL.md @@ -20,7 +20,22 @@ The installation script: - Installs and configures `dnsmasq` (DHCP server for AP mode) - These services can interfere with normal WiFi client mode -### 3. Reboot After Installation +### 3. The Board Ran Out of Memory + +On a 512MB or 1GB board, memory exhaustion stops `sshd` being able to fork a +session process. The connection is accepted and then closed immediately, before +any banner: + +```text +kex_exchange_identification: Connection closed by remote host +``` + +The giveaway is that the board is otherwise healthy — ping is clean and the web +UI still responds — but nothing that needs to start a new process works, and +the panel is usually dark. Only a power cycle clears it. See +[LOW_MEMORY_BOARDS.md](LOW_MEMORY_BOARDS.md). + +### 4. Reboot After Installation If the script reboots the Pi (which it recommends), network services may restart in a different state, potentially triggering AP mode. @@ -190,11 +205,23 @@ The web interface allows you to: ## Summary -**SSH becomes unavailable because**: +**SSH becomes unavailable because** — two unrelated causes, and they need +different responses: + +*AP mode (most common):* - WiFi monitor service enables AP mode when WiFi disconnects - AP mode switches WiFi from client to access point mode - Pi loses connection to your original network +*Memory exhaustion (low-memory boards):* +- The board runs out of memory, so `sshd` cannot fork a session process +- The connection is accepted and closed before any banner +- Ping still answers and the web UI still responds, so it looks healthy +- The panel is usually dark and the service cannot restart +- **Only a power cycle clears this** — there is no remote recovery, because + every remote route needs a new process +- Prevention and tuning: [LOW_MEMORY_BOARDS.md](LOW_MEMORY_BOARDS.md) + **To regain SSH**: 1. Connect to **LEDMatrix-Setup** AP network (password: `ledmatrix123`) 2. SSH to `192.168.4.1` diff --git a/first_time_install.sh b/first_time_install.sh index 2256be52..1a25bb25 100644 --- a/first_time_install.sh +++ b/first_time_install.sh @@ -1688,6 +1688,94 @@ else echo "✗ $CMDLINE_FILE not found; skipping isolcpus optimization" fi +# Enable the memory cgroup controller (idempotent). +# The Pi firmware boots with cgroup_disable=memory, so systemd's MemoryMax= is +# accepted and silently ignored — the display service then has no ceiling, and +# a runaway takes the whole board down (sshd can no longer fork, the panel goes +# dark) rather than just restarting the one service. +if [ "$SKIP_PERF" != "1" ] && [ -f "$CMDLINE_FILE" ]; then + # Both parameters are required for the memory controller, and they can get + # separated -- an image, another tool or a half-applied earlier run can + # leave one without the other. Checking only cgroup_enable=memory would + # report success while MemoryMax= silently does nothing, so each is checked + # and appended independently. + cgroup_missing="" + for cgroup_param in cgroup_enable=memory cgroup_memory=1; do + if ! grep -qw "$cgroup_param" "$CMDLINE_FILE"; then + cgroup_missing="$cgroup_missing $cgroup_param" + fi + done + if [ -z "$cgroup_missing" ]; then + echo "cgroup memory parameters already present in $CMDLINE_FILE" + else + echo "Adding${cgroup_missing} to $CMDLINE_FILE..." + cp "$CMDLINE_FILE" "$CMDLINE_FILE.bak" 2>/dev/null || true + # The kernel command line must stay on one line. + sed -i "1 s|\$|${cgroup_missing}|" "$CMDLINE_FILE" + echo " Takes effect after reboot. Verify with:" + echo " grep memory /sys/fs/cgroup/cgroup.controllers" + fi +fi + +# Persist the journal (idempotent). +# These images default to volatile storage: journald keeps everything in /run +# (tmpfs), so every reboot destroys the logs — including the ones that would +# explain why the board rebooted. Capped so an SD card is not worn out by logs. +# A non-empty /var/log/journal does not prove journald is configured the way +# this needs: the directory survives a switch back to volatile storage, and it +# says nothing about whether a size cap is set. Read the effective +# configuration instead, and only write the keys the user has not set +# themselves so an explicit local limit is preserved. +journald_effective() { + # systemd-analyze merges journald.conf with every drop-in; grep is the + # fallback for images that ship without it. + if command -v systemd-analyze >/dev/null 2>&1 && + systemd-analyze cat-config systemd/journald.conf >/dev/null 2>&1; then + systemd-analyze cat-config systemd/journald.conf 2>/dev/null + else + cat /etc/systemd/journald.conf /etc/systemd/journald.conf.d/*.conf 2>/dev/null + fi +} +journald_conf="$(journald_effective)" +journald_storage="$(printf '%s\n' "$journald_conf" | grep -E '^[[:space:]]*Storage=' | tail -n1 | cut -d= -f2 | tr -d '[:space:]')" +journald_cap="$(printf '%s\n' "$journald_conf" | grep -E '^[[:space:]]*SystemMaxUse=' | tail -n1 | cut -d= -f2 | tr -d '[:space:]')" + +if [ "$journald_storage" = "persistent" ] && [ -n "$journald_cap" ]; then + echo "Persistent journald storage already configured (SystemMaxUse=$journald_cap)" +else + echo "Enabling persistent journald storage..." + mkdir -p /etc/systemd/journald.conf.d + { + echo "# Installed by LEDMatrix first_time_install.sh" + echo "[Journal]" + echo "Storage=persistent" + if [ -n "$journald_cap" ]; then + echo "# SystemMaxUse left to your existing setting ($journald_cap)" + else + # Capped so logs cannot wear out or fill an SD card. + echo "SystemMaxUse=64M" + fi + } > /etc/systemd/journald.conf.d/ledmatrix-persistent.conf + mkdir -p /var/log/journal + systemd-tmpfiles --create --prefix /var/log/journal >/dev/null 2>&1 || true + systemctl restart systemd-journald >/dev/null 2>&1 || true + + # Drop-ins are applied in lexical order, so a locally added file that sorts + # after ledmatrix-persistent.conf (zz-local.conf and friends) still wins. + # Writing the file is not evidence it took effect -- re-read and say so + # plainly rather than reporting success we cannot confirm. + journald_now="$(journald_effective | grep -E '^[[:space:]]*Storage=' | tail -n1 | cut -d= -f2 | tr -d '[:space:]')" + if [ "$journald_now" = "persistent" ]; then + echo " Persistent journald storage active" + else + echo " WARNING: journald storage is still '${journald_now:-unset}' after" + echo " writing /etc/systemd/journald.conf.d/ledmatrix-persistent.conf." + echo " Another drop-in that sorts later is overriding it. Check:" + echo " systemd-analyze cat-config systemd/journald.conf | grep -n Storage=" + echo " Logs will not survive a reboot until that is resolved." + fi +fi + # Ensure dtparam=audio=off in config.txt (idempotent) if [ "$SKIP_PERF" = "1" ]; then : # skipped diff --git a/src/cache/memory_cache.py b/src/cache/memory_cache.py index a13e3ccf..55ca010d 100644 --- a/src/cache/memory_cache.py +++ b/src/cache/memory_cache.py @@ -4,11 +4,58 @@ Memory Cache Handles in-memory caching with TTL support, size limits, and automatic cleanup. """ +import os import time import threading import logging from typing import Dict, Any, Optional +# Historical fixed ceiling, kept as the fallback when RAM cannot be read. +DEFAULT_MAX_SIZE = 1000 + + +def _total_memory_mb() -> Optional[float]: + """Physical RAM in MB, or None where /proc/meminfo is unavailable.""" + try: + with open('/proc/meminfo', 'r', encoding='utf-8') as fh: + for line in fh: + if line.startswith('MemTotal:'): + return int(line.split()[1]) / 1024 + except (OSError, ValueError, IndexError): + return None + return None + + +def default_max_size() -> int: + """Entry ceiling scaled to this machine's RAM. + + One fixed ceiling cannot serve both a 512 MB Pi Zero 2 W and an 8 GB Pi 5. + Entries here are parsed API payloads that routinely run tens of kilobytes + each, so a thousand of them is a comfortable cache on a large board and a + substantial fraction of total RAM on a small one — where the process + competing for that RAM is also driving the panel. Set + LEDMATRIX_CACHE_MAX_ENTRIES to override. + """ + override = os.environ.get('LEDMATRIX_CACHE_MAX_ENTRIES') + if override: + try: + value = int(override) + if value > 0: + return value + except ValueError: + pass + + total_mb = _total_memory_mb() + if total_mb is None: + return DEFAULT_MAX_SIZE + if total_mb < 1536: # 512 MB and 1 GB boards + return 150 + if total_mb < 3072: # 2 GB + return 400 + if total_mb < 6144: # 4 GB + return 800 + return 1500 # 8 GB and up + class MemoryCache: """Manages in-memory cache with TTL and size limits.""" @@ -87,6 +134,32 @@ class MemoryCache: with self._lock: self._cache[key] = value self._timestamps[key] = time.time() + # Enforce the ceiling here rather than leaving it to the periodic + # cleanup, which only runs every cleanup_interval seconds (300 by + # default). A burst of inserts between two sweeps could otherwise + # take the cache far past _max_size, which is the memory growth this + # limit exists to prevent -- and on a 1GB board that is the + # difference between a bounded cache and an unreachable Pi. + self._evict_over_limit_locked() + + def _evict_over_limit_locked(self) -> int: + """Drop oldest entries until the cache is within _max_size. + + Caller must hold self._lock. Returns the number of entries removed. + """ + excess = len(self._cache) - self._max_size + if excess <= 0: + return 0 + oldest = sorted( + self._timestamps.items(), + key=lambda item: float(item[1]) if isinstance(item[1], (int, float)) else 0.0 + ) + removed = 0 + for key, _ in oldest[:excess]: + self._cache.pop(key, None) + self._timestamps.pop(key, None) + removed += 1 + return removed def clear(self, key: Optional[str] = None) -> None: """ @@ -143,22 +216,8 @@ class MemoryCache: self._timestamps.pop(key, None) removed_count += 1 - # Enforce size limit by removing oldest entries if cache is too large - if len(self._cache) > self._max_size: - # Sort by timestamp (oldest first) - sorted_entries = sorted( - self._timestamps.items(), - key=lambda x: float(x[1]) if isinstance(x[1], (int, float)) else 0 - ) - - # Remove oldest entries until we're under the limit - excess_count = len(self._cache) - self._max_size - for i in range(excess_count): - if i < len(sorted_entries): - key = sorted_entries[i][0] - self._cache.pop(key, None) - self._timestamps.pop(key, None) - removed_count += 1 + # Same ceiling enforcement set() uses, so the two cannot drift. + removed_count += self._evict_over_limit_locked() self._last_cleanup = current_time diff --git a/src/cache_manager.py b/src/cache_manager.py index bef98784..67624de1 100644 --- a/src/cache_manager.py +++ b/src/cache_manager.py @@ -33,7 +33,7 @@ import logging import threading import tempfile from src.exceptions import CacheError -from src.cache.memory_cache import MemoryCache +from src.cache.memory_cache import MemoryCache, default_max_size from src.cache.disk_cache import DiskCache from src.cache.cache_strategy import CacheStrategy from src.cache.cache_metrics import CacheMetrics @@ -84,7 +84,9 @@ class CacheManager: self.logger.warning("ConfigManager not available, using default cache intervals") # Initialize cache components using composition - self._memory_cache_component = MemoryCache(max_size=1000, cleanup_interval=300.0) + self._memory_cache_component = MemoryCache( + max_size=default_max_size(), cleanup_interval=300.0 + ) self._disk_cache_component = DiskCache(cache_dir=self.cache_dir, logger=self.logger) self._strategy_component = CacheStrategy(config_manager=self.config_manager, logger=self.logger) self._metrics_component = CacheMetrics(logger=self.logger) diff --git a/src/plugin_system/plugin_health.py b/src/plugin_system/plugin_health.py index ecbec076..d9b4f2c5 100644 --- a/src/plugin_system/plugin_health.py +++ b/src/plugin_system/plugin_health.py @@ -7,7 +7,7 @@ and circuit breaker state. Provides automatic recovery mechanisms. import time import logging -from typing import Dict, Optional, Any +from typing import Dict, Optional, Any, Tuple from enum import Enum @@ -64,10 +64,48 @@ class PluginHealthTracker: cache_key, max_age=None, memory_ttl=0 if force_reload else None ) - if cached: - return cached - - # Default state + if isinstance(cached, dict) and cached: + # Complete it rather than trusting it: a persisted record can be + # missing fields the callers index directly (a partial write, a + # restored backup, an older schema), and returning it verbatim makes + # record_success / record_failure raise KeyError, which takes the + # display down in a restart loop that survives reboots because the + # bad entry is on disk. + state, repaired = self._repair_health_state(cached) + if repaired: + self.logger.warning( + f"Repaired health state for {plugin_id}: " + f"{sorted(repaired)} missing or invalid, using defaults for those." + ) + return state + + # Not a dict at all: written by something other than + # _save_health_state (a key collision, a corrupted entry). Nothing to + # salvage. + if cached is not None and not isinstance(cached, dict): + self.logger.warning( + f"Discarding malformed health state for {plugin_id}: expected " + f"dict, got {type(cached).__name__}. Falling back to defaults." + ) + + return self._default_health_state() + + def _save_health_state(self, plugin_id: str, state: Dict[str, Any]) -> None: + """Save health state to cache.""" + cache_key = self._get_health_key(plugin_id) + self.cache_manager.set(cache_key, state) # Persist indefinitely + self._health_state[plugin_id] = state + + # The fields callers index directly (state['circuit_state'] and friends). + # A cached dict missing any of them raises KeyError deep in record_success / + # record_failure, so the value is completed before it is handed out. + _COUNTER_FIELDS = ('consecutive_failures', 'total_failures', 'total_successes') + _TIMESTAMP_FIELDS = ('last_success_time', 'last_failure_time', + 'circuit_opened_time', 'half_open_start_time') + + @staticmethod + def _default_health_state() -> Dict[str, Any]: + """A fresh state with every field the callers expect.""" return { 'consecutive_failures': 0, 'total_failures': 0, @@ -77,15 +115,56 @@ class PluginHealthTracker: 'circuit_state': CircuitState.CLOSED.value, 'circuit_opened_time': None, 'half_open_start_time': None, - 'last_error': None + 'last_error': None, } - - def _save_health_state(self, plugin_id: str, state: Dict[str, Any]) -> None: - """Save health state to cache.""" - cache_key = self._get_health_key(plugin_id) - self.cache_manager.set(cache_key, state) # Persist indefinitely - self._health_state[plugin_id] = state - + + @classmethod + def _repair_health_state(cls, cached: Dict[str, Any]) -> Tuple[Dict[str, Any], list]: + """Return `cached` completed against the defaults, plus what was repaired. + + Per-field rather than all-or-nothing: a record that has real failure + counts but is missing `last_error` should keep the counts, not be reset + to healthy. Only values that are absent or the wrong type fall back to + the default, so a partial or older-schema record survives with whatever + it does carry, while every field the callers index is guaranteed present + and of a usable type. + """ + state = cls._default_health_state() + repaired = [] + for field, default in state.items(): + if field not in cached: + repaired.append(field) + continue + value = cached[field] + if field in cls._COUNTER_FIELDS: + ok = isinstance(value, int) and not isinstance(value, bool) and value >= 0 + elif field in cls._TIMESTAMP_FIELDS: + # bool is a subclass of int, so True would pass as a timestamp + # and then compare as 1.0 -- expiring a cooldown the instant it + # opens, or (False) making the elapsed check never fire. + ok = value is None or ( + isinstance(value, (int, float)) and not isinstance(value, bool) + ) + elif field == 'circuit_state': + # Membership first requires the value to be hashable: a list or + # dict here would raise TypeError out of the repair itself, + # which is the crash this whole path exists to prevent. + ok = isinstance(value, str) and value in { + member.value for member in CircuitState + } + else: # last_error + ok = value is None or isinstance(value, str) + if ok: + state[field] = value + else: + repaired.append(field) + # Anything the schema has since grown (degraded, degraded_reason) is + # read with .get() by its callers, so carry it through untouched. + for field, value in cached.items(): + if field not in state: + state[field] = value + return state, repaired + def get_health_state(self, plugin_id: str, force_reload: bool = False) -> Dict[str, Any]: """Get current health state for a plugin. diff --git a/src/plugin_system/plugin_loader.py b/src/plugin_system/plugin_loader.py index bd748ef7..d8ae0e3d 100644 --- a/src/plugin_system/plugin_loader.py +++ b/src/plugin_system/plugin_loader.py @@ -14,7 +14,7 @@ import sys import subprocess import threading from pathlib import Path -from typing import Dict, Any, Optional, Tuple, Type +from typing import Dict, Any, List, Optional, Tuple, Type import logging from packaging.requirements import InvalidRequirement, Requirement @@ -45,6 +45,76 @@ def requirements_has_real_deps(requirements_file: str) -> bool: return False +def _extra_dependencies(dist_name: str, extras) -> Optional[List[Requirement]]: + """Dependencies a distribution declares *only* behind the given extras. + + Returns None when the installed metadata cannot be read or parsed, so the + caller can fall back to running pip rather than assuming anything. + """ + try: + meta = importlib.metadata.metadata(dist_name) + except importlib.metadata.PackageNotFoundError: + return None + + gated: List[Requirement] = [] + for raw in meta.get_all('Requires-Dist') or []: + try: + dep = Requirement(raw) + except InvalidRequirement: + return None + if dep.marker is None: + continue + # Keep only what the distribution gates behind an extra we asked for: + # satisfied when `extra` is that name, but not when no extra is + # requested. A marker that holds either way (python_version, sys_platform) + # belongs to the base install and is already covered by the version check. + if dep.marker.evaluate({'extra': ''}): + continue + if any(dep.marker.evaluate({'extra': extra}) for extra in extras): + gated.append(dep) + return gated + + +def _extras_are_satisfied(req: Requirement, _visited: Optional[set] = None) -> bool: + """Check the dependencies pulled in by req's extras are installed. + + Follows extras through nested extras. A gated dependency can itself request + one (`requests[socks]`), and checking only that `requests` is installed at + an acceptable version says nothing about whether the socks extra's own + dependency is there -- so the caller would skip pip and the plugin would + fail at import instead. Plain dependencies are still checked one level + deep, which is all that is needed to tell "the extra was installed" from + "the extra was never installed". + + `_visited` carries the (distribution, extras) pairs already seen, so a + dependency cycle between extras terminates instead of recursing forever. + Anything unreadable returns False, so the caller still falls through to pip. + """ + if _visited is None: + _visited = set() + marker = (req.name.lower(), frozenset(e.lower() for e in req.extras)) + if marker in _visited: + # Already accounted for higher up the chain; treating a cycle as + # satisfied here is safe because the outer frame still has to pass. + return True + _visited.add(marker) + + gated = _extra_dependencies(req.name, req.extras) + if gated is None: + return False + + for dep in gated: + try: + dep_version = importlib.metadata.version(dep.name) + except importlib.metadata.PackageNotFoundError: + return False + if dep.specifier and not dep.specifier.contains(dep_version, prereleases=True): + return False + if dep.extras and not _extras_are_satisfied(dep, _visited): + return False + return True + + def requirements_are_satisfied(requirements_file: str) -> bool: """ Check whether every real requirement line in requirements.txt is already @@ -76,9 +146,6 @@ def requirements_are_satisfied(requirements_file: str) -> bool: except InvalidRequirement: return False - if req.extras: - return False # verifying extras' sub-dependencies isn't worth it here - if req.marker is not None and not req.marker.evaluate(): continue # not applicable on this platform/interpreter @@ -90,6 +157,9 @@ def requirements_are_satisfied(requirements_file: str) -> bool: if req.specifier and not req.specifier.contains(installed_version, prereleases=True): return False + if req.extras and not _extras_are_satisfied(req): + return False + return True diff --git a/systemd/ledmatrix-wifi-monitor.service b/systemd/ledmatrix-wifi-monitor.service index 2f08fde0..46b69bfe 100644 --- a/systemd/ledmatrix-wifi-monitor.service +++ b/systemd/ledmatrix-wifi-monitor.service @@ -10,8 +10,8 @@ WorkingDirectory=__PROJECT_ROOT_DIR__ ExecStart=/usr/bin/python3 __PROJECT_ROOT_DIR__/scripts/utils/wifi_monitor_daemon.py --interval 30 Restart=on-failure RestartSec=10 -StandardOutput=syslog -StandardError=syslog +StandardOutput=journal +StandardError=journal SyslogIdentifier=ledmatrix-wifi-monitor [Install] diff --git a/systemd/ledmatrix.service b/systemd/ledmatrix.service index f3bc9e28..d5f064b6 100644 --- a/systemd/ledmatrix.service +++ b/systemd/ledmatrix.service @@ -9,8 +9,25 @@ User=root WorkingDirectory=__PROJECT_ROOT_DIR__ Environment=PYTHONDONTWRITEBYTECODE=1 ExecStart=/usr/bin/python3 __PROJECT_ROOT_DIR__/run.py -Restart=on-failure +# Restart=always, not on-failure: run.py exiting 0 (a clean shutdown path taken +# for a reason that no longer applies, e.g. a config reload) would otherwise leave +# the service stopped and the panel dark indefinitely, with systemd considering +# that a successful outcome and never bringing it back. +Restart=always RestartSec=10 +# Memory ceiling as a share of physical RAM, so one unit file suits a 512 MB +# Pi Zero 2 W and an 8 GB Pi 5 alike. This is a backstop, not a tuning knob: it +# turns "the board runs out of memory, stops being able to fork, and takes sshd +# and the panel down together until someone pulls the plug" into "this one +# service restarts". +# +# NOTE: Raspberry Pi firmware boots the kernel with cgroup_disable=memory, and +# systemd accepts this setting and then silently ignores it. Verify with: +# grep memory /sys/fs/cgroup/cgroup.controllers +# If that prints nothing, add "cgroup_enable=memory cgroup_memory=1" to +# /boot/firmware/cmdline.txt (all on line 1) and reboot. first_time_install.sh +# does this for you. +MemoryMax=85% StandardOutput=journal StandardError=journal SyslogIdentifier=ledmatrix diff --git a/test/test_cache_manager.py b/test/test_cache_manager.py index 8a93a39d..9d1195a8 100644 --- a/test/test_cache_manager.py +++ b/test/test_cache_manager.py @@ -458,3 +458,26 @@ class TestDiskCacheWriteEconomy: cache = DiskCache(cache_dir=str(tmp_path)) cache.set("k", {"when": datetime(2026, 7, 12, 10, 30)}) assert cache.get("k") == {"when": "2026-07-12T10:30:00"} + + +# --- the ceiling has to hold between cleanup sweeps --------------------------- + +def test_memory_cache_enforces_ceiling_on_every_write(): + """_cleanup_memory_cache only runs every cleanup_interval seconds (300 by + default). If set() accepted entries without bound in between, a burst could + take the cache far past max_size -- which is the unbounded growth the limit + exists to prevent, and on a 1GB board the difference between a bounded cache + and a Pi that cannot fork. + """ + from src.cache.memory_cache import MemoryCache + + cache = MemoryCache(max_size=150, cleanup_interval=300.0) + for i in range(1000): + cache.set(f"k{i}", {"v": i}) + + assert len(cache._cache) <= 150 + # The timestamp map has to be evicted alongside the values, or it becomes + # the leak instead. + assert len(cache._timestamps) <= 150 + assert cache.get("k999") is not None, "the newest write must survive" + assert cache.get("k0") is None, "the oldest must be the one evicted" diff --git a/test/test_plugin_health.py b/test/test_plugin_health.py index fd647a4b..f418061c 100644 --- a/test/test_plugin_health.py +++ b/test/test_plugin_health.py @@ -91,3 +91,101 @@ def test_force_reload_refreshes_stale_in_memory_snapshot(): # and it asked the cache to bypass the in-memory tier (memory_ttl=0). assert any(c.kwargs.get("memory_ttl") == 0 for c in cache.get.call_args_list) + + +# --- persisted state that does not match the current schema ------------------- +# +# A record on disk can be missing fields the callers index directly: a partial +# write, a restored backup, or a state written by an older schema. Returning it +# verbatim raises KeyError inside record_success / record_failure, which takes +# the display down in a restart loop that survives reboots, because the bad +# entry is on disk and gets read again on the way back up. Observed in the wild +# as `plugin clock-simple operation failed: 'circuit_state'`, repeating ~50x a +# minute with the panel frozen. + +_INDEXED_FIELDS = ( + "consecutive_failures", "total_failures", "total_successes", + "last_success_time", "last_failure_time", "circuit_state", + "circuit_opened_time", "half_open_start_time", "last_error", +) + + +def _tracker_reading(persisted): + cache = _cache() + cache.get.return_value = persisted + return PluginHealthTracker(cache) + + +def test_partial_state_is_completed_not_returned_raw(): + """The shape seen in the wild: one field, everything else absent.""" + state = _tracker_reading({"circuit_state": "closed"}).get_health_state("p") + for field in _INDEXED_FIELDS: + assert field in state, f"{field} missing; callers index it directly" + + +def test_repair_keeps_real_failure_history(): + """A record with genuine counts must not be reset to healthy just because + an optional field is absent -- that would clear a tripped breaker.""" + state = _tracker_reading({ + "consecutive_failures": 5, + "total_failures": 5, + "circuit_state": "open", + }).get_health_state("p") + assert state["consecutive_failures"] == 5 + assert state["total_failures"] == 5 + assert state["circuit_state"] == "open" + + +def test_wrong_types_fall_back_per_field(): + """A counter persisted as a string would pass a membership check and then + fail on the first += 1; an unknown circuit_state would take a branch the + breaker has no handling for.""" + state = _tracker_reading({ + "consecutive_failures": "3", + "circuit_state": "melted", + "total_failures": 7, + }).get_health_state("p") + assert state["consecutive_failures"] == 0 + assert state["circuit_state"] == CircuitState.CLOSED.value + assert state["total_failures"] == 7, "valid neighbours must survive" + + +def test_newer_fields_are_carried_through(): + """degraded/degraded_reason are read with .get() and are not part of the + indexed set; repairing must not drop them.""" + state = _tracker_reading({ + "circuit_state": "closed", "degraded": True, "degraded_reason": "x", + }).get_health_state("p") + assert state["degraded"] is True + assert state["degraded_reason"] == "x" + + +def test_recording_against_a_repaired_state_does_not_raise(): + """The actual failure: record_failure indexing a field that was not there. + + The seed deliberately omits circuit_state. Seeding a record that *has* it + would pass against the old raw-return behaviour too -- the counters are + read with .get(), so circuit_state is the only field whose absence used to + raise. + """ + tracker = _tracker_reading({"total_failures": 2, "total_successes": 1}) + tracker.record_failure("p", Exception("boom")) + tracker.record_success("p") + + +def test_unhashable_or_boolean_values_are_repaired(): + """Values that break the repair itself rather than a later caller. + + An unhashable circuit_state raises TypeError inside a set membership test, + and bool is a subclass of int, so True would pass as a timestamp and then + compare as 1.0 -- expiring a cooldown the moment it opens. + """ + for bad_state in ({"circuit_state": []}, {"circuit_state": {}}): + state = _tracker_reading(bad_state).get_health_state("p") + assert state["circuit_state"] == CircuitState.CLOSED.value + + state = _tracker_reading({ + "circuit_opened_time": True, "last_success_time": False, + }).get_health_state("p") + assert state["circuit_opened_time"] is None + assert state["last_success_time"] is None