fix(graphrag): bound SignalStore.LogClusters (close the last in-memory RAM leak)#120
Merged
Conversation
LogClusters was the one unbounded structure on the in-memory hot path: an insert-only map keyed by service×Drain-template-ID, explicitly skipped by Prune. Template IDs shift as Drain generalizes and the key carries the service, so the map grew with every distinct (service, template-id) pair ever seen — unbounded by the shared 50k Drain LRU and by log volume. Under a sustained high-shape log stream (exactly the case when INFO is kept in-memory but not persisted) this leaks RAM. Extend SignalStore.Prune to bound LogClusters the same way it bounds Metrics: - TTL: evict clusters with LastSeen older than the 24h signalRetention cutoff (ages out the template-ID accretion as orphaned clusters go stale). - Cap: maxLogClustersPerTenant=10000 backstop, oldest-LastSeen first. - Evicted clusters' EMITTED_BY / LOGGED_DURING edges are swept (collected by cluster id; removed even when not yet time-stale, e.g. cap-evicted). Prune signature gains maxLogClusters; refresh.go passes the new constant. Tests: rewrote _SweepsStaleLogClusterEdgesOnly (which asserted the old never-pruned behavior) into _DropsStaleLogClustersKeepsFresh; added _LogClusterCapEvictsOldest. Verified: go build/vet, golangci-lint exit 0, go test -race ./internal/graphrag (109), full suite 679, gofmt clean. Closes the last unbounded structure on the in-memory path. CPU stays bounded by the fixed 16-worker pool + drop-when-full event queue; RAM by GOMEMLIMIT (75% budget) + per-tenant caps/TTLs on TraceStore (500k+TTL), SignalStore metrics (2k) + now log clusters (10k), topology LRU (100k), TSDB cardinality, and 24h idle-tenant eviction. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDQVJrixs2nJoea67a8pEG
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Bounds
SignalStore.LogClusters— the one unbounded structure found on the in-memory hot path during a CPU/RAM audit.It was an insert-only map keyed by
lc_{service}_{drain-template-id}, explicitly skipped byPrune(with a comment claiming it was "bounded upstream by the Drain template LRU"). That's only partly true: the key carries the service (so N services multiply it) and template IDs shift as Drain generalizes, so each evolving log shape mints new keys while the oldLogClusterNodes are never removed. Net: the map grows with every distinct(service, template-id)pair ever seen — unbounded by the shared 50k Drain LRU and by log volume.This is precisely the structure exercised when INFO logs are kept in-memory but not stored (the current
STORE_MIN_SEVERITY=WARNdefault), under a high-log-shape-cardinality stream.Fix
Extend
SignalStore.Pruneto boundLogClustersexactly like it already boundsMetrics:signalRetentioncutoff. This is what kills the accretion leak: orphaned (stale template-id) clusters age out.maxLogClustersPerTenant=10000backstop, oldest-LastSeenfirst.EMITTED_BY/LOGGED_DURINGedges are swept (collected by cluster id, so cap-evicted-but-still-fresh clusters don't leave dangling edges).CPU/RAM picture (the question this answers)
With this merged, the in-memory path is fully bounded:
GOMEMLIMIT(75% of cgroup/host budget) paces GC, under per-tenant caps+TTLs on TraceStore (500k + 30m/1h TTL), SignalStore metrics (2k) + now log clusters (10k), the topology LRU (100k), TSDB cardinality, and 24h idle-tenant eviction. Designed per-tenant ceiling ≈ 200–300 MB (TraceStore-dominated).Verification
go build/vet,golangci-lintexit 0,go test -race ./internal/graphrag(109), full suite 679 pass / 28 pkgs, gofmt clean. Rewrote the test that asserted the old never-pruned behavior; added a cap-eviction test.🤖 Generated with Claude Code