From 3a55246d2ae655bbe4d20661d4c3020cb733f712 Mon Sep 17 00:00:00 2001 From: Chuck Date: Sun, 12 Jul 2026 12:05:07 -0400 Subject: [PATCH] fix(cache): stop fsync-hammering the SD card on unchanged data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DiskCache.set wrote every key as mkstemp -> json.dump(indent=4) -> flush+fsync -> replace -> chmod, on the persistent cache dir — dozens of force-flushed SD writes per minute on an API-heavy install, mostly rewriting identical data every plugin update cycle. - Serialize once, compact (no indent): cache files are machine-read only; indenting multiplied the bytes written. - Skip the disk when the payload for a key is unchanged (adler32 map, per-process); refresh the file mtime instead so records relying on mtime for TTL don't expire early. Self-heals if the file was removed externally (expiry cleanup). - Drop the per-write fsync: os.replace already guarantees readers never see a torn file, and cache data is re-fetchable — the flush bought nothing but card wear. API unchanged; DateTimeEncoder round-trip covered by tests. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam --- src/cache/disk_cache.py | 57 +++++++++++++++++++++++++++++++------ test/test_cache_manager.py | 58 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 9 deletions(-) diff --git a/src/cache/disk_cache.py b/src/cache/disk_cache.py index a1a525ad..64b72f07 100644 --- a/src/cache/disk_cache.py +++ b/src/cache/disk_cache.py @@ -10,6 +10,7 @@ import tempfile import logging import threading +import zlib from typing import Dict, Any, Optional, Protocol from datetime import datetime @@ -53,6 +54,11 @@ def __init__(self, cache_dir: Optional[str], logger: Optional[logging.Logger] = self.cache_dir = cache_dir self.logger = logger or logging.getLogger(__name__) self._lock = threading.Lock() + # key -> adler32 of the last payload successfully written to the + # primary cache path; lets set() skip rewriting identical data + # (per-process only — worst case another process rewrites, never + # a missed write). Guarded by _lock. + self._write_digests: Dict[str, int] = {} def get_cache_path(self, key: str) -> Optional[str]: """ @@ -155,10 +161,35 @@ def set(self, key: str, data: Dict[str, Any]) -> None: cache_path = self.get_cache_path(key) if not cache_path: return - + + # Serialize once, compact (no indent): the payload is reused by every + # write path below, and cache files are machine-read only — indenting + # them just multiplied the bytes written to the SD card. + try: + payload = json.dumps(data, cls=DateTimeEncoder) + except (TypeError, ValueError) as e: + self.logger.warning("Cache data for key '%s' not serializable: %s", key, e) + return + + digest = zlib.adler32(payload.encode('utf-8')) + try: # Atomic write to avoid partial/corrupt files with self._lock: + # Skip the disk entirely when this exact payload was already + # written for this key (plugins re-save unchanged API data + # every update cycle — each write is real SD-card wear). + # Refresh the file mtime so records that rely on it for TTL + # (no embedded 'timestamp') don't expire early; a metadata + # touch is journal-cheap compared to rewriting the data. + if self._write_digests.get(key) == digest: + try: + os.utime(cache_path, None) + return + except OSError: + # File vanished or perms changed — fall through and write + self._write_digests.pop(key, None) + tmp_dir = os.path.dirname(cache_path) # Try to create temp file in cache directory first # If that fails due to permissions, fall back to direct write @@ -181,13 +212,17 @@ def set(self, key: str, data: Dict[str, Any]) -> None: fd = None if tmp_path and fd is not None: - # Use atomic write with temp file + # Atomic write with temp file. No fsync: os.replace + # already guarantees readers never see a torn file, + # and cache data is re-fetchable — forcing a disk + # flush per write was the single biggest SD-card + # wear source (dozens of fsyncs/min on API-heavy + # installs) for data that can be re-downloaded. try: with os.fdopen(fd, 'w', encoding='utf-8') as tmp_file: - json.dump(data, tmp_file, indent=4, cls=DateTimeEncoder) - tmp_file.flush() - os.fsync(tmp_file.fileno()) + tmp_file.write(payload) os.replace(tmp_path, cache_path) + self._write_digests[key] = digest # Set proper permissions: 660 (rw-rw----) for group-readable cache files try: os.chmod(cache_path, 0o660) # nosec B103 - intentional; web UI and service share a group @@ -203,9 +238,8 @@ def set(self, key: str, data: Dict[str, Any]) -> None: # Fallback: direct write (not atomic, but better than failing) try: with open(cache_path, 'w', encoding='utf-8') as cache_file: - json.dump(data, cache_file, indent=4, cls=DateTimeEncoder) - cache_file.flush() - os.fsync(cache_file.fileno()) + cache_file.write(payload) + self._write_digests[key] = digest # Set proper permissions: 660 (rw-rw----) for group-readable cache files try: os.chmod(cache_path, 0o660) # nosec B103 - intentional; web UI and service share a group @@ -229,9 +263,12 @@ def set(self, key: str, data: Dict[str, Any]) -> None: pass if os.path.isdir(fallback_dir) and os.access(fallback_dir, os.W_OK): + # NOTE: no digest record here — the fallback file + # is a different path, so future sets must keep + # retrying the primary location. fallback_path = os.path.join(fallback_dir, os.path.basename(cache_path)) with open(fallback_path, 'w', encoding='utf-8') as tmp_file: - json.dump(data, tmp_file, indent=4, cls=DateTimeEncoder) + tmp_file.write(payload) # Set proper permissions: 660 (rw-rw----) for group-readable cache files try: os.chmod(fallback_path, 0o660) # nosec B103 - intentional; web UI and service share a group @@ -272,6 +309,7 @@ def clear(self, key: Optional[str] = None) -> None: with self._lock: if key: + self._write_digests.pop(key, None) cache_path = self.get_cache_path(key) if cache_path and os.path.exists(cache_path): try: @@ -280,6 +318,7 @@ def clear(self, key: Optional[str] = None) -> None: self.logger.warning("Could not remove cache file %s: %s", cache_path, e) else: # Clear all cache files + self._write_digests.clear() if os.path.exists(self.cache_dir): for filename in os.listdir(self.cache_dir): if filename.endswith('.json'): diff --git a/test/test_cache_manager.py b/test/test_cache_manager.py index 63a8d2e6..8a93a39d 100644 --- a/test/test_cache_manager.py +++ b/test/test_cache_manager.py @@ -400,3 +400,61 @@ def test_multiple_fetch_times(self): assert stats['fetch_count'] == 3 assert stats['total_fetch_time'] == 1.8 assert stats['average_fetch_time'] == pytest.approx(0.6, abs=0.01) + + +class TestDiskCacheWriteEconomy: + """SD-card wear guards: identical payloads skip the disk, files are + compact, and TTL semantics survive the skip (see PR: fix/diskcache-sd-wear).""" + + def test_identical_set_skips_rewrite(self, tmp_path): + import os + cache = DiskCache(cache_dir=str(tmp_path)) + cache.set("k", {"data": "v"}) + path = cache.get_cache_path("k") + first = os.stat(path) + os.utime(path, (first.st_atime - 100, first.st_mtime - 100)) # age it + aged_mtime = os.stat(path).st_mtime + ino_before = os.stat(path).st_ino + cache.set("k", {"data": "v"}) # identical payload + after = os.stat(path) + # mtime refreshed (TTL for mtime-based records preserved)... + assert after.st_mtime > aged_mtime + # ...but the file was NOT rewritten (same inode: no replace happened) + assert after.st_ino == ino_before + + def test_changed_data_rewrites(self, tmp_path): + import os + cache = DiskCache(cache_dir=str(tmp_path)) + cache.set("k", {"data": "v1"}) + cache.set("k", {"data": "v2"}) + assert cache.get("k") == {"data": "v2"} + + def test_clear_resets_digest(self, tmp_path): + import os + cache = DiskCache(cache_dir=str(tmp_path)) + cache.set("k", {"data": "v"}) + cache.clear("k") + assert cache.get("k") is None + cache.set("k", {"data": "v"}) # same payload after clear must WRITE + assert cache.get("k") == {"data": "v"} + + def test_skip_self_heals_when_file_deleted_externally(self, tmp_path): + import os + cache = DiskCache(cache_dir=str(tmp_path)) + cache.set("k", {"data": "v"}) + os.remove(cache.get_cache_path("k")) # e.g. expiry cleanup + cache.set("k", {"data": "v"}) # digest matches but file is gone + assert cache.get("k") == {"data": "v"} + + def test_files_are_compact_json(self, tmp_path): + cache = DiskCache(cache_dir=str(tmp_path)) + cache.set("k", {"a": 1, "b": [1, 2, 3]}) + raw = open(cache.get_cache_path("k")).read() + assert "\n" not in raw.strip() # no indent + assert cache.get("k") == {"a": 1, "b": [1, 2, 3]} + + def test_datetime_round_trip_still_works(self, tmp_path): + from datetime import datetime + 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"}