Skip to content

Stop hourly spike detection loading every pattern's error_data - #719

Merged
tonyalaribe merged 1 commit into
masterfrom
fix/spike-detection-heap
Oct 25, 2026
Merged

tonyalaribe merged 1 commit into
masterfrom
fix/spike-detection-heap

Conversation

@tonyalaribe

Copy link
Copy Markdown
Contributor

Fixes the hourly monoscope-server restarts (heap overflow at the -M14G limit, one replica at ~:01 most hours since 2026-10-22).

Cause: a7ece43 made the hourly per-project jobs run in prod for the first time. ErrorSpikeDetection → getErrorPatternsWithCurrentRates selected error_data for every open pattern: 4,221 rows / 4.27 GB of JSON for the self-monitoring project (HasqlException messages embedding whole SQL statements; max 21 MB a row). Container RSS went 2.25 → 9.9 GB in 30 s before the crash.

Fix:

  • Drop error_data from ErrorPatternWithCurrentRate and the query (spike detection never used it).
  • Cap message/rootErrorMessage (4,000) and stackTrace (16,000) when patterns are upserted; the hash is already computed, so grouping is unchanged.

Not in this PR: trimming the existing oversized error_data rows (a data migration).

Tests: ErrorPatterns specs 35/35, including spike detection and the regress→escalate cycle.

🤖 Generated with Claude Code

Since a7ece43 made the hourly per-project jobs actually run, ErrorSpikeDetection
loaded error_data for every open pattern: 4.3 GB of JSON for the self-monitoring
project (HasqlException messages embedding whole SQL statements, up to 21 MB a row),
overflowing the 14G heap and restarting a replica at ~:01 most hours since 10-22.
The query no longer selects it (nothing used it), and stored message/root message/
stack trace are capped so a pattern can't grow that large again.
@tonyalaribe
tonyalaribe merged commit 65e5849 into master Oct 25, 2026
5 checks passed
@tonyalaribe
tonyalaribe deleted the fix/spike-detection-heap branch October 25, 2026 22:21
@claude

claude Bot commented Oct 25, 2026

Copy link
Copy Markdown

Review — fix is correct and minimal; root cause (loading error_data for every open pattern hourly) is addressed directly. Notes:

  1. Missing regression test for the cap. The only test change adapts the spec to the removed field. Per the bug-fix workflow, add a test that upserts an error with a >4000-char message/rootErrorMessage and >16000-char stackTrace, then asserts the stored error_data is truncated and the pattern still groups under the same hash. Without it, the bounded cap can silently regress.
  2. Truncation by T.take on Text is fine (code points, no mid-codepoint split), but note it applies before HI.AsJsonb encoding, so JSON-escaped size can still be larger (e.g. quotes/newlines in SQL) — acceptable, just not a hard byte bound.
  3. Succinctness: bounded can use one lens-free update via record dot/generic-lens as you did; consider a tiny caps constants (4000, 16000) only if reused — otherwise fine inline. The three field @"…" %~ T.take n could be folded: e & field @"message" %~ T.take 4000 & field @"rootErrorMessage" %~ T.take 4000 is already tight; no change needed.
  4. Inconsistent caps: the read query still uses LEFT(e.message, 2000) / LEFT(e.stacktrace, 8000) while writes cap at 4000/16000. Harmless, but a single pair of numbers would be easier to reason about.
  5. Existing oversized rows remain (acknowledged as out of scope). Other readers of error_data per pattern (e.g. getErrorPatternById, lists) can still pull 21 MB rows until the data trim lands — worth a follow-up ticket.
  6. The SQL -- comment inside the query is shipped to the DB on every call; a Haskell comment above the query would be leaner.

No security concerns. LGTM once (1) is added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant