From 05fbcb09dd117a05c8d629f7576152fabe5b61d8 Mon Sep 17 00:00:00 2001 From: Kpa-clawbot Date: Sun, 5 Apr 2026 12:49:46 -0700 Subject: [PATCH] fix: wire cacheTTL.analyticsHashSizes config to collision cache (#420) (#622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #420 — wires `cacheTTL` config values to server-side cache durations that were previously hardcoded. ## Problem `collisionCacheTTL` was hardcoded at 60s in `store.go`. The config has `cacheTTL.analyticsHashSizes: 3600` (1 hour) but it was never read — the `/api/config/cache` endpoint just passed the raw map to the client without applying values server-side. ## Changes - **`store.go`**: Add `cacheTTLSec()` helper to safely extract duration values from the `cacheTTL` config map. `NewPacketStore` now accepts an optional `cacheTTL` map (variadic, backward-compatible) and wires: - `cacheTTL.analyticsHashSizes` → `collisionCacheTTL` - `cacheTTL.analyticsRF` → `rfCacheTTL` - **Default changed**: `collisionCacheTTL` default raised from 60s → 3600s (1 hour). Hash collision computation is expensive and data changes rarely — 60s was causing unnecessary recomputation. - **`main.go`**: Pass `cfg.CacheTTL` to `NewPacketStore`. - **Tests**: Added `TestCacheTTLFromConfig` and `TestCacheTTLDefaults` in eviction_test.go. Updated existing `TestHashCollisionsCacheTTL` for the new default. ## Audit of other cacheTTL values The remaining `cacheTTL` keys (`stats`, `nodeDetail`, `nodeHealth`, `nodeList`, `bulkHealth`, `networkStatus`, `observers`, `channels`, `channelMessages`, `analyticsTopology`, `analyticsChannels`, `analyticsSubpaths`, `analyticsSubpathDetail`, `nodeAnalytics`, `nodeSearch`, `invalidationDebounce`) are **client-side only** — served via `/api/config/cache` and consumed by the frontend. They don't have corresponding server-side caches to wire to. The only server-side caches (`rfCache`, `topoCache`, `hashCache`, `chanCache`, `distCache`, `subpathCache`, `collisionCache`) all use either `rfCacheTTL` or `collisionCacheTTL`, both now configurable. ## Complexity O(1) config lookup at store init time. No hot-path impact. Co-authored-by: you --- cmd/server/eviction_test.go | 26 +++++++++++++++++++++++++ cmd/server/main.go | 2 +- cmd/server/routes_test.go | 6 +++--- cmd/server/store.go | 39 +++++++++++++++++++++++++++++++++++-- 4 files changed, 67 insertions(+), 6 deletions(-) diff --git a/cmd/server/eviction_test.go b/cmd/server/eviction_test.go index 9cb5ace6..94aaa6e0 100644 --- a/cmd/server/eviction_test.go +++ b/cmd/server/eviction_test.go @@ -276,3 +276,29 @@ func TestNewPacketStoreNilConfig(t *testing.T) { t.Fatalf("expected retentionHours=0, got %f", store.retentionHours) } } + +func TestCacheTTLFromConfig(t *testing.T) { + // With config values: analyticsHashSizes and analyticsRF should override defaults. + cacheTTL := map[string]interface{}{ + "analyticsHashSizes": float64(7200), + "analyticsRF": float64(300), + } + store := NewPacketStore(nil, nil, cacheTTL) + if store.collisionCacheTTL != 7200*time.Second { + t.Fatalf("expected collisionCacheTTL=7200s, got %v", store.collisionCacheTTL) + } + if store.rfCacheTTL != 300*time.Second { + t.Fatalf("expected rfCacheTTL=300s, got %v", store.rfCacheTTL) + } +} + +func TestCacheTTLDefaults(t *testing.T) { + // Without config, defaults should apply. + store := NewPacketStore(nil, nil) + if store.collisionCacheTTL != 3600*time.Second { + t.Fatalf("expected default collisionCacheTTL=3600s, got %v", store.collisionCacheTTL) + } + if store.rfCacheTTL != 15*time.Second { + t.Fatalf("expected default rfCacheTTL=15s, got %v", store.rfCacheTTL) + } +} diff --git a/cmd/server/main.go b/cmd/server/main.go index 48177487..a70c0531 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -139,7 +139,7 @@ func main() { } // In-memory packet store - store := NewPacketStore(database, cfg.PacketStore) + store := NewPacketStore(database, cfg.PacketStore, cfg.CacheTTL) if err := store.Load(); err != nil { log.Fatalf("[store] failed to load: %v", err) } diff --git a/cmd/server/routes_test.go b/cmd/server/routes_test.go index 75e669fe..83a54775 100644 --- a/cmd/server/routes_test.go +++ b/cmd/server/routes_test.go @@ -3186,7 +3186,7 @@ func TestHashCollisionsClassification(t *testing.T) { } func TestHashCollisionsCacheTTL(t *testing.T) { - // Issue #420: collision cache should use dedicated TTL (60s), not rfCacheTTL (15s) + // Issue #420: collision cache should use dedicated TTL, default 3600s (1 hour) db := setupTestDB(t) seedTestData(t, db) store := NewPacketStore(db, nil) @@ -3194,8 +3194,8 @@ func TestHashCollisionsCacheTTL(t *testing.T) { t.Fatalf("store.Load failed: %v", err) } - if store.collisionCacheTTL != 60*time.Second { - t.Errorf("expected collisionCacheTTL=60s, got %v", store.collisionCacheTTL) + if store.collisionCacheTTL != 3600*time.Second { + t.Errorf("expected collisionCacheTTL=3600s, got %v", store.collisionCacheTTL) } if store.rfCacheTTL != 15*time.Second { t.Errorf("expected rfCacheTTL=15s, got %v", store.rfCacheTTL) diff --git a/cmd/server/store.go b/cmd/server/store.go index 2bf1eb39..2991dec3 100644 --- a/cmd/server/store.go +++ b/cmd/server/store.go @@ -207,8 +207,33 @@ type cachedResult struct { expiresAt time.Time } +// cacheTTLSec extracts a duration from the cacheTTL config map. +// Values may be float64 (from JSON) or int. Returns false if key is missing or non-positive. +func cacheTTLSec(m map[string]interface{}, key string) (time.Duration, bool) { + v, ok := m[key] + if !ok { + return 0, false + } + var sec float64 + switch n := v.(type) { + case float64: + sec = n + case int: + sec = float64(n) + case int64: + sec = float64(n) + default: + return 0, false + } + if sec <= 0 { + return 0, false + } + return time.Duration(sec * float64(time.Second)), true +} + // NewPacketStore creates a new empty packet store backed by db. -func NewPacketStore(db *DB, cfg *PacketStoreConfig) *PacketStore { +// cacheTTLs is the optional cacheTTL map from config.json; keys are strings, values are seconds. +func NewPacketStore(db *DB, cfg *PacketStoreConfig, cacheTTLs ...map[string]interface{}) *PacketStore { ps := &PacketStore{ db: db, packets: make([]*StoreTx, 0, 65536), @@ -229,7 +254,7 @@ func NewPacketStore(db *DB, cfg *PacketStoreConfig) *PacketStore { distCache: make(map[string]*cachedResult), subpathCache: make(map[string]*cachedResult), rfCacheTTL: 15 * time.Second, - collisionCacheTTL: 60 * time.Second, + collisionCacheTTL: 3600 * time.Second, invCooldown: 10 * time.Second, spIndex: make(map[string]int, 4096), spTxIndex: make(map[string][]*StoreTx, 4096), @@ -239,6 +264,16 @@ func NewPacketStore(db *DB, cfg *PacketStoreConfig) *PacketStore { ps.retentionHours = cfg.RetentionHours ps.maxMemoryMB = cfg.MaxMemoryMB } + // Wire cacheTTL config values to server-side cache durations. + if len(cacheTTLs) > 0 && cacheTTLs[0] != nil { + ct := cacheTTLs[0] + if v, ok := cacheTTLSec(ct, "analyticsHashSizes"); ok { + ps.collisionCacheTTL = v + } + if v, ok := cacheTTLSec(ct, "analyticsRF"); ok { + ps.rfCacheTTL = v + } + } return ps }