feat(anomaly): replication output-buffer pressure detector (valkey#3963)

[jamby77]

Detector #4 of the board batch — the observability half of upstream's approved throttling umbrella (valkey#3963): surface replica output-buffer pressure before the client-output-buffer-limit slave overflow forces a disconnect + full-resync loop, and name the loop once it's live.

Detection (per poll, primaries with replicas only)

• CLIENT LIST TYPE replica → per-replica omem vs the parsed slave <hard> <soft> <secs> triplet:
  • WARNING at ≥60% of the hard limit, or ≥60% of the soft limit sustained beyond <secs> (soft-limit expiry also disconnects)
  • CRITICAL at ≥90% of the hard limit
• Resync-loop confirmation: a sync_full increment coinciding with a recently-pressured replica → CRITICAL (the overrun already happened). An increment with no prior pressure never fires — fresh-replica/restart syncs are legitimate.
• slave 0 0 0 (unlimited): ratio alerts skipped; repeated full syncs within the pressure-memory window still fire the loop signal (memory pressure instead).

Hysteresis & degradation

• Client-saturation-style level map per replica (escalation-only emits, re-arm below the warning threshold), addr-keyed so reconnects keep hysteresis; stored level advances only after a successful emit, so a failed emit retries next poll.
• CLIENT LIST or CONFIG GET denied → degrades to the mem_clients_slaves aggregate ratio (message notes reduced fidelity) + the sync_full delta; the poll never crashes.
• Limit triplet cached with a slow recheck cadence (runtime CONFIG SET picked up; failed reads retry next poll rather than pinning a value).
• Standalone / zero-replica connections add zero command overhead.

Wiring

New MetricType.REPL_BUFFER_PRESSURE (state-based): buffer-loop exclusion, dashboard label. Per-replica Prometheus gauge betterdb_repl_output_buffer_ratio{connection,replica} with stale-label removal — mirrors to OTLP via the existing registry mirror (#319). All state cleaned in onConnectionRemoved.

Tests

21 pure-module cases (triplet parser, thresholds, soft-limit clock, hysteresis/ack/re-arm, unlimited, coincidence vs fresh-sync, pressure-memory expiry, aggregate fallback, pruning) + 7 service-integration cases (poll-path WARNING, steady-state dedupe, sync_full CRITICAL, gauge update, zero-cost no-op, permission-denied resilience, cleanup). Full anomaly-detection suite: 242 green. tsc clean on api; eslint clean on the touched web file.

Advisory copy: "Replica <addr> output buffer at N% of the client-output-buffer-limit slave hard limit — approaching a forced disconnect + full resync (valkey#3963). Raise the limit or reduce the write burst."

---

Note

Medium Risk
New primary-side polling uses CONFIG GET and CLIENT LIST on replication paths; failures are handled gracefully but mis-tuned limits or permissions could affect alert fidelity. Logic is complex (hysteresis, resync correlation) with broad test coverage.

Overview
Adds replica output-buffer (COB) pressure detection for Valkey primaries (valkey#3963): warn before client-output-buffer-limit slave forces a disconnect/resync loop, and raise CRITICAL when sync_full aligns with recent pressure.

A new cob-pressure-detector module parses the slave limit triplet, compares per-replica omem from CLIENT LIST TYPE replica (or mem_clients_slaves when denied), applies escalation-only hysteresis, soft-limit grace, and resync-loop logic with refire cooldowns. AnomalyService.detectCobPressure runs on each poll for masters only, caches CONFIG with slow recheck, skips work when there are no replicas, and clears state/gauges on demotion or connection removal.

MetricType.REPL_BUFFER_PRESSURE is registered as state-based (outside z-score buffering). Prometheus exposes betterdb_repl_output_buffer_ratio per connection/replica via updateReplBufferPressure with stale-label cleanup. The anomaly dashboard maps repl_buffer_pressure to “Replica Buffer Pressure”. Unit and integration tests cover the detector and poll path.

^{Reviewed by Cursor Bugbot for commit 752fbe1. Bugbot is set up for automated code reviews on this repo. Configure here.}

[jamby77]

bugbot run

[jamby77]

bugbot run

[jamby77]

bugbot run

[jamby77]

bugbot run

[jamby77]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

3 issues from previous reviews remain unresolved.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 5b34f56. Configure here.}

[jamby77]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit be2357e. Configure here.}

[KIvanow]

The core is careful and correct - triplet parsing, addr-keyed escalation-only hysteresis, and the per-replica sync_full coincidence-vs-overflow discrimination are all well done and well-tested. One design bug blocks merge though:

Blocking:

1. Unlimited-COB path fires uncorrelated CRITICALs. When the limit is slave 0 0 0, the resync-loop branch fires CRITICAL on any 2 sync_full increments in 5 min with zero pressure correlation. But slave 0 0 0 is a recommended operator mitigation for resync loops and common on large primaries - so rolling replica restarts, 2+ replicas doing initial sync at startup, or a replica flapping on an unrelated network blip all trip a CRITICAL. The per-replica path discriminates coincidence from overflow; the unlimited path has none. Please gate it behind pressure correlation (or at minimum a cooldown). Note the current test asserts this as intended - that expectation should change with the fix.
2. Resync-loop alerts have no dedup. These findings bypass hysteresis (acknowledgeCobFinding no-ops them) and addAnomaly has no cooldown for this metric type, so a sustained loop emits one fresh CRITICAL per poll. Needs a re-emission gate.

Should fix:
3. Soft-sustained warning message says "soft-limit expiry forces a disconnect + full resync," but it fires at 60% of softBytes while the server only disconnects at 100% held past <secs>. It's an early-warning heuristic wearing an imminent-disconnect message - please soften the wording.
4. The permission-degrade is narrower than the doc-comment claims: the mem_clients_slaves aggregate fallback requires limit !== null, so if CONFIG GET is denied it falls into the unlimited branch and emits the "unlimited (or unreadable)" message for a node that actually has a finite limit.

Nit: ~90% of the anomaly.service.spec.ts diff is unrelated Prettier reflow of the Raft tests - would be cleaner as a separate formatting commit.

Verdict: needs changes - the unlimited-COB false-positive + per-poll spam are the merge blockers; the rest is minor.

[jamby77]

Addressed in 2187a69:

1. Unlimited-COB false positives — the no-ratio path now correlates against replica-count growth: a sync_full increment matched by connected_slaves growth in the same interval (a join, a rolling restart coming back, replicas doing initial sync at startup) is explained and doesn't count toward the loop threshold. Only unexplained increments — churn without net replica growth, the loop signature — accumulate. Tests updated/added: startup joins, rolling restart, and the burst case now asserts the gated behavior.
2. Re-emission gate — acknowledging a resync-loop finding now closes a 5-min re-emission gate (COB_RESYNC_REFIRE_MS), covering both the correlated per-replica path and the no-ratio path. It's deliberately connection-global rather than per-addr: a loop's reconnects arrive on fresh ephemeral ports, so an addr-keyed gate would never suppress anything.
3. Soft-sustained wording — now explicitly an early warning ("if it crosses the soft limit and stays there for Ns, the server forces a disconnect"), and includes the soft-limit bytes for context.
4. Permission degrade — the CONFIG-GET-denied case now emits its own message ("could not be read (CONFIG GET denied), so buffer pressure could not be correlated") instead of claiming the limit is unlimited, and the service doc-comment now states the real degrade ladder (CLIENT LIST denied → aggregate, CONFIG GET denied → growth-gated sync_full only).

On the Prettier-reflow nit: the reflow is prettier --write output (repo guidance is to run it on touched files), so splitting it out would reintroduce non-prettier formatting on this branch. Left as is — shout if you'd rather I carve it into a separate formatting commit anyway.

[jamby77]

Conflicts resolved — squashed the branch to one commit and rebased onto current master (which now includes #345/#346/#338 from today). Resolution highlights: the COB state maps, cleanup, and detector method now sit alongside the merged failover-churn code; REPL_BUFFER_PRESSURE joined METRICS_HANDLED_OUTSIDE_EXTRACTOR; and both the churn and COB service-spec describes coexist. 276 anomaly tests green across all five suites, tsc clean. GitHub reports MERGEABLE — ready for the stack-UI merge. #348 has been rebased on top (squashed too) with the FAILOVER_CHURN corroborator re-added now that #346 is in, per the review plan.