mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 14:25:08 +00:00
refactor(cache): remove the cache layer's duplicate cleanup and dead lookups (#613)
* refactor(cache): collapse CacheStrategy's all-60 defaults table and twin soccer branch get_sport_live_interval() without a config manager looked the sport up in a table where every value was 60, with 60 as the fallback; it now returns 60. get_data_type_from_key() had an `if 'soccer'` branch returning the same 'sports_live' as its else. test_cache_strategy_intervals pins the returned strategy for every data type x sport key x config-manager shape; it passes unchanged on the old code. A 2,544-entry dump of every CacheStrategy method over a wider grid is identical before and after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(cache): drop CacheStrategy's `<sport>_scoreboard` config lookup get_sport_live_interval() and get_cache_strategy() read live/recent/ upcoming intervals from config[f"{sport}_scoreboard"]. Those sections belonged to the built-in scoreboards the plugin system replaced; plugin config is keyed by plugin id ("football-scoreboard"), so on a current config the lookup always fell through to the defaults (60 live, 1800 recent, 10800 upcoming), which are now returned directly. The one input where this differs: a config.json upgraded from the pre-plugin era that still carries e.g. an "nfl_scoreboard" section (no code removes them), queried with an explicit sport key. No caller in core or the plugin monorepo passes a sport key here -- get_with_auto_strategy only derives one for keys classed sports_live/live_scores, and its callers (odds managers, odds-ticker) use odds keys -- so the stale section was unreachable in practice. A dump of every CacheStrategy method over 2,544 inputs differs from the previous commit only in those 45 legacy-config entries; the test grid now includes that shape. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf(cache): list cache files without holding the memory-tier lock CacheManager.list_cache_files() held the in-memory cache's lock while it listed and stat'd the whole cache directory -- 8,864 files on a real rig -- so every get()/set() from the display loop and plugins waited out the scan. The lock never protected the disk: DiskCache writes and deletes under their own lock, and a file vanishing between listdir and stat was already handled (logged and skipped). The body is unchanged apart from the dedent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(cache): delegate memory-tier cleanup and stats to MemoryCache CacheManager._cleanup_memory_cache() was a line-for-line copy of MemoryCache.cleanup(), and get_memory_cache_stats() a copy of MemoryCache.get_stats(), both reaching into the component's private _cache/_timestamps/_lock through "backward compatibility" aliases bound in __init__. So the component's own cleanup and stats only ever ran in tests, and the aliases went stale whenever the component was swapped (test_cache_ttl_honoured does). Both now delegate, and the aliases are gone: nothing in core, the tests, or the ledmatrix-plugins monorepo reads them. Behaviour is the same. Compared line by line, the two cleanups differ only in the sort key's fallback (0 vs 0.0, which orders identically), range+bounds check vs slice for the eviction, and the logger name on the DEBUG summary line (src.cache_manager -> src.cache.memory_cache). A differential run over 20,000 random memory states (str/None/garbage/ future timestamps, orphan keys, sizes 0-12, forced and throttled runs) gives identical removed counts, resulting dicts and last-cleanup times; the same harness catches each of three seeded mutations of MemoryCache.cleanup. The throttle clock also moves with it: CacheManager kept its own copy of last-cleanup, the component's is used now, and they started equal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(background): inline the sport cache key and drop the unused request queue get_sport_cache_key() constructed a whole CacheManager -- ConfigManager, config parse, cache-dir probing with test-file writes -- to return f"{sport}_{date}". It now builds the key itself in the same format as CacheManager.generate_sport_cache_key() (UTC date, %Y%m%d); tests check the two agree for explicit dates and, with a frozen clock at 03:30 UTC, for the default date. Median per call on Windows: ~0.6 ms -> ~2 us (alternating runs); on a Pi the old path also wrote a probe file per call. request_queue was a PriorityQueue nothing ever put into: requests go straight to the executor, so `priority` never did anything. The queue is gone; the `priority` parameter and FetchRequest field stay (every monorepo scoreboard passes priority=) and are documented as ignored, and get_statistics() keeps reporting queue_size, now a literal 0 as it always was in practice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Chuck
+55
-124
@@ -92,14 +92,6 @@ class CacheManager:
|
||||
self._strategy_component = CacheStrategy(config_manager=self.config_manager, logger=self.logger)
|
||||
self._metrics_component = CacheMetrics(logger=self.logger)
|
||||
|
||||
# Keep old attributes for backward compatibility (delegated to components)
|
||||
self._memory_cache = self._memory_cache_component._cache
|
||||
self._memory_cache_timestamps = self._memory_cache_component._timestamps
|
||||
self._cache_lock = self._memory_cache_component._lock
|
||||
self._max_memory_cache_size = self._memory_cache_component._max_size
|
||||
self._memory_cache_cleanup_interval = self._memory_cache_component._cleanup_interval
|
||||
self._last_memory_cache_cleanup = self._memory_cache_component._last_cleanup
|
||||
|
||||
# Disk cleanup configuration
|
||||
self._disk_cleanup_interval_hours = 24 # Run cleanup every 24 hours
|
||||
self._disk_cleanup_interval = 3600.0 # Minimum interval between cleanups (1 hour) for throttle
|
||||
@@ -229,70 +221,14 @@ class CacheManager:
|
||||
return None
|
||||
|
||||
def _cleanup_memory_cache(self, force: bool = False) -> int:
|
||||
"""
|
||||
Clean up expired entries from memory cache and enforce size limits.
|
||||
|
||||
Args:
|
||||
force: If True, perform cleanup regardless of time interval
|
||||
|
||||
"""Sweep the memory tier: drop entries older than an hour and trim it
|
||||
to its size ceiling, at most once per cleanup interval unless forced.
|
||||
|
||||
Returns:
|
||||
Number of entries removed
|
||||
"""
|
||||
now = time.time()
|
||||
|
||||
# Check if cleanup is needed
|
||||
if not force and (now - self._last_memory_cache_cleanup) < self._memory_cache_cleanup_interval:
|
||||
return 0
|
||||
|
||||
with self._cache_lock:
|
||||
removed_count = 0
|
||||
current_time = time.time()
|
||||
|
||||
# Remove expired entries (entries older than 1 hour without access are considered expired)
|
||||
# We use a conservative TTL of 1 hour for cleanup
|
||||
max_age_for_cleanup = 3600 # 1 hour
|
||||
|
||||
expired_keys = []
|
||||
for key, timestamp in list(self._memory_cache_timestamps.items()):
|
||||
if isinstance(timestamp, str):
|
||||
try:
|
||||
timestamp = float(timestamp)
|
||||
except ValueError:
|
||||
timestamp = None
|
||||
|
||||
if timestamp is None or (current_time - timestamp) > max_age_for_cleanup:
|
||||
expired_keys.append(key)
|
||||
|
||||
# Remove expired entries
|
||||
for key in expired_keys:
|
||||
self._memory_cache.pop(key, None)
|
||||
self._memory_cache_timestamps.pop(key, None)
|
||||
removed_count += 1
|
||||
|
||||
# Enforce size limit by removing oldest entries if cache is too large
|
||||
if len(self._memory_cache) > self._max_memory_cache_size:
|
||||
# Sort by timestamp (oldest first)
|
||||
sorted_entries = sorted(
|
||||
self._memory_cache_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._memory_cache) - self._max_memory_cache_size
|
||||
for i in range(excess_count):
|
||||
if i < len(sorted_entries):
|
||||
key = sorted_entries[i][0]
|
||||
self._memory_cache.pop(key, None)
|
||||
self._memory_cache_timestamps.pop(key, None)
|
||||
removed_count += 1
|
||||
|
||||
self._last_memory_cache_cleanup = current_time
|
||||
|
||||
if removed_count > 0:
|
||||
self.logger.debug(f"Memory cache cleanup: removed {removed_count} entries (current size: {len(self._memory_cache)})")
|
||||
|
||||
return removed_count
|
||||
|
||||
return self._memory_cache_component.cleanup(force=force)
|
||||
|
||||
def _get_cache_path(self, key: str) -> Optional[str]:
|
||||
"""Get the path for a cache file."""
|
||||
return self._disk_cache_component.get_cache_path(key)
|
||||
@@ -412,56 +348,58 @@ class CacheManager:
|
||||
current_time = time.time()
|
||||
|
||||
try:
|
||||
with self._cache_lock:
|
||||
for filename in os.listdir(self.cache_dir):
|
||||
if not filename.endswith('.json'):
|
||||
continue
|
||||
# No lock: this is disk-only work, and the memory-tier lock it used
|
||||
# to hold would stall every get/set while thousands of files are
|
||||
# stat'd. A file deleted mid-scan is skipped below.
|
||||
for filename in os.listdir(self.cache_dir):
|
||||
if not filename.endswith('.json'):
|
||||
continue
|
||||
|
||||
# Extract key from filename (remove .json extension)
|
||||
key = filename[:-5] # Remove '.json'
|
||||
|
||||
file_path = os.path.join(self.cache_dir, filename)
|
||||
|
||||
try:
|
||||
# Get file stats
|
||||
stat_info = os.stat(file_path)
|
||||
size_bytes = stat_info.st_size
|
||||
modified_time = stat_info.st_mtime
|
||||
age_seconds = current_time - modified_time
|
||||
|
||||
# Extract key from filename (remove .json extension)
|
||||
key = filename[:-5] # Remove '.json'
|
||||
# Format age display
|
||||
if age_seconds < 60:
|
||||
age_display = f"{int(age_seconds)}s"
|
||||
elif age_seconds < 3600:
|
||||
age_display = f"{int(age_seconds / 60)}m"
|
||||
elif age_seconds < 86400:
|
||||
age_display = f"{int(age_seconds / 3600)}h"
|
||||
else:
|
||||
age_display = f"{int(age_seconds / 86400)}d"
|
||||
|
||||
file_path = os.path.join(self.cache_dir, filename)
|
||||
# Format size display
|
||||
if size_bytes < 1024:
|
||||
size_display = f"{size_bytes}B"
|
||||
elif size_bytes < 1024 * 1024:
|
||||
size_display = f"{size_bytes / 1024:.1f}KB"
|
||||
else:
|
||||
size_display = f"{size_bytes / (1024 * 1024):.1f}MB"
|
||||
|
||||
cache_files.append({
|
||||
'key': key,
|
||||
'filename': filename,
|
||||
'age_seconds': age_seconds,
|
||||
'age_display': age_display,
|
||||
'size_bytes': size_bytes,
|
||||
'size_display': size_display,
|
||||
'path': file_path,
|
||||
'modified_time': modified_time,
|
||||
'modified_datetime': datetime.fromtimestamp(modified_time).isoformat()
|
||||
})
|
||||
except OSError as e:
|
||||
self.logger.warning(f"Error getting stats for cache file {filename} at {file_path}: {e}", exc_info=True)
|
||||
continue
|
||||
|
||||
try:
|
||||
# Get file stats
|
||||
stat_info = os.stat(file_path)
|
||||
size_bytes = stat_info.st_size
|
||||
modified_time = stat_info.st_mtime
|
||||
age_seconds = current_time - modified_time
|
||||
|
||||
# Format age display
|
||||
if age_seconds < 60:
|
||||
age_display = f"{int(age_seconds)}s"
|
||||
elif age_seconds < 3600:
|
||||
age_display = f"{int(age_seconds / 60)}m"
|
||||
elif age_seconds < 86400:
|
||||
age_display = f"{int(age_seconds / 3600)}h"
|
||||
else:
|
||||
age_display = f"{int(age_seconds / 86400)}d"
|
||||
|
||||
# Format size display
|
||||
if size_bytes < 1024:
|
||||
size_display = f"{size_bytes}B"
|
||||
elif size_bytes < 1024 * 1024:
|
||||
size_display = f"{size_bytes / 1024:.1f}KB"
|
||||
else:
|
||||
size_display = f"{size_bytes / (1024 * 1024):.1f}MB"
|
||||
|
||||
cache_files.append({
|
||||
'key': key,
|
||||
'filename': filename,
|
||||
'age_seconds': age_seconds,
|
||||
'age_display': age_display,
|
||||
'size_bytes': size_bytes,
|
||||
'size_display': size_display,
|
||||
'path': file_path,
|
||||
'modified_time': modified_time,
|
||||
'modified_datetime': datetime.fromtimestamp(modified_time).isoformat()
|
||||
})
|
||||
except OSError as e:
|
||||
self.logger.warning(f"Error getting stats for cache file {filename} at {file_path}: {e}", exc_info=True)
|
||||
continue
|
||||
|
||||
except OSError as e:
|
||||
self.logger.error(f"Error listing cache directory {self.cache_dir}: {e}", exc_info=True)
|
||||
return []
|
||||
@@ -1001,14 +939,7 @@ class CacheManager:
|
||||
Returns:
|
||||
Dictionary with memory cache statistics
|
||||
"""
|
||||
with self._cache_lock:
|
||||
return {
|
||||
'size': len(self._memory_cache),
|
||||
'max_size': self._max_memory_cache_size,
|
||||
'usage_percent': (len(self._memory_cache) / self._max_memory_cache_size * 100) if self._max_memory_cache_size > 0 else 0,
|
||||
'last_cleanup': self._last_memory_cache_cleanup,
|
||||
'cleanup_interval': self._memory_cache_cleanup_interval
|
||||
}
|
||||
return self._memory_cache_component.get_stats()
|
||||
|
||||
def log_memory_cache_stats(self) -> None:
|
||||
"""Log current memory cache statistics."""
|
||||
|
||||
Reference in New Issue
Block a user