From fdd788212d5ded6c515351c32e81c19f2d59a0a8 Mon Sep 17 00:00:00 2001 From: TJ Downes <273720+tjdownes@users.noreply.github.com> Date: Tue, 21 Apr 2026 19:55:38 -0700 Subject: [PATCH 1/2] perf(rrdtool): cache get_data() result for 60 s to avoid repeated disk reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem ------- rrdtool.fetch() is a blocking C library call that reads 24 hours of RRD data from disk. The dashboard can call get_data() on every page refresh. On an SD card each fetch can cost several milliseconds of I/O, and because the RRD step is 60 seconds the data cannot change more often than that — any fetch within the same 60-second window returns identical data. The combined-optimizations branch had a 60-second read cache; rightup's batching refactor inadvertently removed it. This PR restores it. Solution -------- * Add self._get_data_cache: tuple = (0.0, None) to __init__ * In get_data(): set use_cache = (start_time is None and end_time is None) - if use_cache and cache is < 60 s old: return cached result immediately - after a successful live fetch with use_cache: store (now, result) * Explicit start_time / end_time callers always bypass the cache so fine-grained or historical queries are never stale Why 60 s TTL? The RRD step is 60 s, so the database cannot hold a newer sample until the next step boundary. A 60-second cache is tight enough that the dashboard always shows data ≤ one step stale, and loose enough that a burst of refreshes costs one disk read instead of N. Co-Authored-By: Claude Sonnet 4.6 --- repeater/data_acquisition/rrdtool_handler.py | 21 +++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/repeater/data_acquisition/rrdtool_handler.py b/repeater/data_acquisition/rrdtool_handler.py index a1e2397..3a66f87 100644 --- a/repeater/data_acquisition/rrdtool_handler.py +++ b/repeater/data_acquisition/rrdtool_handler.py @@ -23,6 +23,10 @@ class RRDToolHandler: self._pending_rrd_update = None self._last_rrd_info_time = 0 self._last_rrd_info_cache = None + # Read-side cache: rrdtool.fetch() returns 24 h of data and is a + # blocking disk read. Cache the result for 60 s — matching the RRD + # step size — so repeated dashboard refreshes don't hammer the SD card. + self._get_data_cache: tuple = (0.0, None) # (fetched_at, result) def _init_rrd(self): if not self.available: @@ -162,9 +166,20 @@ class RRDToolHandler: ) return None + # Serve from cache if result is still fresh. RRD step is 60 s, so + # anything newer than that is guaranteed to be identical to a live fetch. + # Only the default (full 24-hour, no explicit bounds) call is cached — + # explicit start/end requests always bypass the cache. + now = time.time() + use_cache = start_time is None and end_time is None + if use_cache: + cache_fetched_at, cache_result = self._get_data_cache + if now - cache_fetched_at < 60.0 and cache_result is not None: + return cache_result + try: if end_time is None: - end_time = int(time.time()) + end_time = int(now) if start_time is None: start_time = end_time - (24 * 3600) @@ -220,6 +235,10 @@ class RRDToolHandler: result["timestamps"] = timestamps + # Populate read cache for default (unconstrained) calls only. + if use_cache: + self._get_data_cache = (now, result) + return result except Exception as e: From d592af6e19195b6f04336929f619c808b8c8d92e Mon Sep 17 00:00:00 2001 From: TJ Downes <273720+tjdownes@users.noreply.github.com> Date: Tue, 21 Apr 2026 20:50:27 -0700 Subject: [PATCH 2/2] fix(rrdtool): replace rrdtool.info() with self-tracked timestamp to eliminate allocation storm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem ------- update_packet_metrics() called rrdtool.info() (cached for 5 s) to get the RRD's last_update timestamp. rrdtool.info() returns a massive Python dict: 17 data sources × 5 RRAs × ~8 fields each = ~700+ dict entries per call. tracemalloc showed +10696 new allocations / +251 KB at this exact line, flagged as "Investigate" in the memory diagnostics dashboard. The rrdtool.info() approach was also unnecessarily complex: it required a 5-second secondary cache, a _pending_rrd_update buffer, and two extra instance attributes — all to answer one question ("did we already write this period?") that we can answer ourselves with a single integer. Fix --- Replace _last_rrd_info_cache / _last_rrd_info_time / _pending_rrd_update with a single self._last_rrd_update: int = 0 that stores the timestamp of the last successful rrdtool.update() call. The throttle check becomes: if timestamp <= self._last_rrd_update: return On success: self._last_rrd_update = timestamp Zero dict allocations per call. The only downside vs rrdtool.info() is that _last_rrd_update resets to 0 on process restart, meaning the first packet after a restart always triggers a write — correct behaviour. Co-Authored-By: Claude Sonnet 4.6 --- repeater/data_acquisition/rrdtool_handler.py | 45 ++++++-------------- 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/repeater/data_acquisition/rrdtool_handler.py b/repeater/data_acquisition/rrdtool_handler.py index 3a66f87..ca075aa 100644 --- a/repeater/data_acquisition/rrdtool_handler.py +++ b/repeater/data_acquisition/rrdtool_handler.py @@ -19,10 +19,10 @@ class RRDToolHandler: self.rrd_path = self.storage_dir / "metrics.rrd" self.available = RRDTOOL_AVAILABLE self._init_rrd() - # Batch RRD updates: track pending update and last cached info - self._pending_rrd_update = None - self._last_rrd_info_time = 0 - self._last_rrd_info_cache = None + # Timestamp of the last successful rrdtool.update() call (unix seconds, + # aligned to the 60-second RRD step). Used to skip writes whose period + # has already been committed — no rrdtool.info() call needed. + self._last_rrd_update: int = 0 # Read-side cache: rrdtool.fetch() returns 24 h of data and is a # blocking disk read. Cache the result for 60 s — matching the RRD # step size — so repeated dashboard refreshes don't hammer the SD card. @@ -81,10 +81,11 @@ class RRDToolHandler: logger.error(f"Failed to create RRD database: {e}") def update_packet_metrics(self, record: dict, cumulative_counts: dict): - """Buffer packet metrics for batch RRD update instead of per-packet writes. - - RRD uses 60-second time steps, so we batch updates within each period - and only write when the time period changes or buffer is full. + """Write packet metrics to RRD, throttled to once per 60-second step. + + RRD enforces a 60-second minimum step between updates. We track the + last written timestamp ourselves — no rrdtool.info() call needed, which + previously allocated thousands of Python objects per call. """ if not self.available or not self.rrd_path.exists(): return @@ -92,27 +93,8 @@ class RRDToolHandler: try: timestamp = int(record.get("timestamp", time.time())) - # Cache RRD info for up to 5 seconds to avoid repeated rrdtool.info() calls - now = time.time() - if now - self._last_rrd_info_time > 5 or self._last_rrd_info_cache is None: - try: - self._last_rrd_info_cache = rrdtool.info(str(self.rrd_path)) - self._last_rrd_info_time = now - except Exception as e: - logger.debug(f"Failed to cache RRD info: {e}") - self._last_rrd_info_cache = None - return - - if self._last_rrd_info_cache is None: - return - - last_update = int(self._last_rrd_info_cache.get("last_update", timestamp - 60)) - - # Skip if timestamp is in same or earlier time period than last update - # (RRD step is 60 seconds) - if timestamp <= last_update: - # But still buffer cumulative counts for when we do update - self._pending_rrd_update = (timestamp, cumulative_counts, record) + # Skip if this packet falls in the same 60-second period we already wrote. + if timestamp <= self._last_rrd_update: return # Build update string from cumulative counts @@ -144,11 +126,8 @@ class RRDToolHandler: type_values_str = ":".join(type_values) values = f"{basic_values}:{type_values_str}" - # Write to RRD - this is now only called once per 60-second period rrdtool.update(str(self.rrd_path), values) - # Invalidate cache so next period fetches fresh info - self._last_rrd_info_cache = None - self._pending_rrd_update = None + self._last_rrd_update = timestamp except Exception as e: logger.error(f"Failed to update RRD packet metrics: {e}")