Skip to content

fix(brief-dedup): cap replay-log lists to prevent Upstash 500MB record limit#3649

Merged
koala73 merged 1 commit into
mainfrom
fix/digest-replay-log-ltrim-cap
May 10, 2026
Merged

fix(brief-dedup): cap replay-log lists to prevent Upstash 500MB record limit#3649
koala73 merged 1 commit into
mainfrom
fix/digest-replay-log-ltrim-cap

Conversation

@koala73

@koala73 koala73 commented May 10, 2026

Copy link
Copy Markdown
Owner

Summary

Found the source of today's Upstash "Max Record Size 500MB hit 6× in 5 minutes" alerts. The digest:replay-log:v1:full:en:all:<date> lists have been growing unbounded within each day — the largest at 421,716 entries × ~1.5KB each = ~630MB per key, well over the 500MB Fixed-plan record-size limit.

Likely connected to today's seed-forecasts crash

The seed-forecasts cron failed at 13:00 UTC with a Redis SET timeout on a 0.17MB payload (well under any limit). When Upstash's backend struggles with a 630MB-record write, it back-pressures adjacent operations. Trimming the oversized lists should reduce that pressure.

Evidence

Sweep of all replay-log keys:

421716 entries  digest:replay-log:v1:full:en:all:2026-05-07
419782 entries  digest:replay-log:v1:full:en:all:2026-05-08
419473 entries  digest:replay-log:v1:full:en:all:2026-05-06
411401 entries  digest:replay-log:v1:full:en:all:2026-05-05
397556 entries  digest:replay-log:v1:full:en:all:2026-04-30
... 10 more daily logs at 100K-360K each

Root cause

scripts/lib/brief-dedup-replay-log.mjs:323 issued only RPUSH + EXPIRE, no LTRIM. The 30d TTL bounded cross-day rotation, but within a single busy day the list grew without limit:

const rpushCmd = ['RPUSH', key, ...records.map((r) => JSON.stringify(r))];
const expireCmd = ['EXPIRE', key, String(TTL_SECONDS)];
const result = await pipelineImpl([rpushCmd, expireCmd]);
//                                       ↑ no LTRIM

Fix

Insert LTRIM key -100000 -1 between RPUSH and EXPIRE — tail-keep the most recent 100,000 entries:

const rpushCmd = ['RPUSH', key, ...records.map((r) => JSON.stringify(r))];
const ltrimCmd = ['LTRIM', key, `-${REPLAY_LOG_MAX_ENTRIES_PER_DAY}`, '-1'];
const expireCmd = ['EXPIRE', key, String(TTL_SECONDS)];

Cap chosen: 100,000 × ~1.5KB ≈ 150MB → 3× safety margin under 500MB. Cap exported as REPLAY_LOG_MAX_ENTRIES_PER_DAY so a future bump only needs one location change.

Why tail-keep is safe for the use-case

The U6 14-day replay harness aggregates ACROSS days using stable repHash cluster identity (per the existing comments in replay-digest-cooldown.mjs). Within-day older-entry eviction is acceptable for calibration analysis — operators get a representative sample of each day's traffic, not exhaustive coverage. The newest 100K still vastly exceeds any realistic sweep sample size.

Test plan

  • node --test tests/brief-dedup-replay-log.test.mjs — 31/31 pass
    • Updated existing pipeline-shape assertion (2 commands → 3, with LTRIM in the middle)
    • Added regression guard reading the cap from the exported REPLAY_LOG_MAX_ENTRIES_PER_DAY constant (no brittle dual-update)
    • Sanity assertion that the cap stays in the 10K-200K band (so future bumps stay safely under 500MB at observed entry sizes)
  • Post-merge: monitor Upstash for "Max Record Size" alerts → expectation: zero new alerts on digest:replay-log:* after the next writer cycle
  • Post-merge: monitor seed-forecasts cron success rate → expectation: improved publish reliability

One-time cleanup of EXISTING oversized keys

The new code only caps NEW writes. Existing keys are already at 200K-420K entries. They'll naturally trim when the next writer pass adds even one entry (the LTRIM applies to the whole list, not just new entries). But to immediately reclaim the ~6GB of overage, run this after merge:

TOKEN=$UPSTASH_REDIS_REST_TOKEN
URL=$UPSTASH_REDIS_REST_URL
for d in 04-25 04-26 04-27 04-28 04-29 04-30 05-01 05-02 05-03 05-04 05-05 05-06 05-07 05-08 05-09 05-10; do
  for variant in full finance markets safety conflict;  do
    k="digest:replay-log:v1:${variant}:en:all:2026-${d}"
    curl -s "$URL/ltrim/$k/-100000/-1" -H "Authorization: Bearer $TOKEN"
    echo "  trimmed $k"
  done
done

(Or wait for next writer cycle — same end state, just slower.)

Companion to today's other resilience work

Upstash alerted today: "Max Record Size limit of 500MB hit at least 6
times in the last 5 minutes." Scanned the keyspace and found the
offender: `digest:replay-log:v1:full:en:all:<date>` lists at 200K-420K
entries each (~1.5KB per entry → up to ~630MB per key, exceeding the
500MB Fixed-plan limit).

Root cause: the writer in scripts/lib/brief-dedup-replay-log.mjs:323
issued RPUSH + EXPIRE but no LTRIM cap. The 30d TTL bounded
cross-day rotation but within a single busy day, the list grew
unbounded.

Fix: append LTRIM `-N..-1` (tail-keep) between RPUSH and EXPIRE so
each per-day key is bounded to the most recent N entries. Single
extra pipeline command — minimal cost.

Cap chosen: 100,000 entries × ~1.5KB ≈ 150MB → ~3× safety margin
under 500MB. The U6 14-day replay harness already aggregates ACROSS
days using stable repHash cluster identity, so within-day eviction of
older entries is acceptable for the calibration use-case (operators
get a representative sample, not exhaustive coverage).

Likely connected to today's seed-forecasts publish timeout: Upstash
backend struggling on these huge-record writes back-pressures
adjacent operations. The 0.17MB forecasts payload itself was fine,
but the SET timed out at the 15s ceiling. Trimming the oversized
lists should reduce that pressure too.

Tests:
- Updated existing pipeline-shape assertion (2 commands → 3, with
  LTRIM in the middle)
- Added regression guard that reads the cap from the exported
  REPLAY_LOG_MAX_ENTRIES_PER_DAY constant so a future bump only
  needs one location change
- Sanity assertion that the cap stays in the 10K-200K band

31/31 tests pass. No behavioral change for the consumer side
(replay/sweep tools): they read via LRANGE, which sees the bounded
list transparently. Older entries are still retained for the day's
most-recent 100K, which exceeds any realistic sample size for
calibration analysis.

Companion: 15 existing keys are already over the cap (largest at
421K entries). They'll be trimmed by the next writer pass, OR you
can force-trim them now via a manual LTRIM sweep — see PR comments.
@vercel

vercel Bot commented May 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
worldmonitor Ready Ready Preview, Comment May 10, 2026 3:44pm

Request Review

@greptile-apps

greptile-apps Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Inserts an LTRIM command into the Upstash Redis pipeline that writes per-day replay-log lists, capping each list at 100,000 entries (~150MB) to stay well under Upstash's 500MB max-record-size limit. Tests are updated to assert the three-command pipeline shape and a new regression guard verifies the LTRIM offset is read from the exported REPLAY_LOG_MAX_ENTRIES_PER_DAY constant.

  • REPLAY_LOG_MAX_ENTRIES_PER_DAY = 100_000 is exported so a future cap change only touches one location; the test reads the constant directly to avoid brittle hard-coding.
  • The LTRIM key -N -1 (tail-keep) placement after RPUSH and before EXPIRE means all records added in the current tick survive the trim (single-tick volume is always far below 100K), and the TTL is still refreshed on every write.

Confidence Score: 5/5

Safe to merge — the change is a targeted, well-scoped addition of a single LTRIM command to an existing Redis pipeline, with no impact on the digest delivery path.

The fix is minimal: one new exported constant and one extra pipeline command inserted in the correct position (after RPUSH, before EXPIRE). Tail-keep semantics ensure all records pushed in the current tick survive the trim. No other call sites are affected, and the best-effort write contract means even a hypothetical LTRIM error cannot impact digest delivery.

No files require special attention. Both changed files are straightforward and well-tested.

Important Files Changed

Filename Overview
scripts/lib/brief-dedup-replay-log.mjs Adds REPLAY_LOG_MAX_ENTRIES_PER_DAY constant (exported) and inserts LTRIM into the Redis pipeline. Logic is correct; minor note that wrote returns records pushed this tick, not post-trim list size, but this is fine in practice since a single tick will never exceed the 100K cap.
tests/brief-dedup-replay-log.test.mjs Existing pipeline-shape test updated from 2 to 3 commands; new regression guard imports the constant directly to avoid dual-maintenance; sanity bounds (10K-200K) prevent a future unsafe bump. Test coverage is solid.

Reviews (1): Last reviewed commit: "fix(brief-dedup): cap replay-log lists t..." | Re-trigger Greptile

@koala73
koala73 merged commit cfe00fa into main May 10, 2026
12 checks passed
@koala73
koala73 deleted the fix/digest-replay-log-ltrim-cap branch May 10, 2026 15:59
koala73 added a commit that referenced this pull request May 10, 2026
…3650)

* feat(seed-utils): retry transient Upstash failures in atomicPublish

WM 2026-05-10 incident: seed-forecasts cron failed with a Redis SET
timeout on a 0.17MB payload after completing all expensive work
(LLM calls, 42 forecasts filtered, payload built). Stack trace:

  DOMException [TimeoutError]: The operation was aborted due to timeout
    at async redisCommand (_seed-utils.mjs:140:16)
    at async atomicPublish (_seed-utils.mjs:254:5)
    at async runSeed (_seed-utils.mjs:1236:27)

A single transient Upstash REST timeout at the FINAL publish step
killed a 3-minute LLM-heavy run. Cron just waited an hour for the
next tick; forecasts + marketImplications were stale for ~3h.

Fix: wrap atomicPublish's body (staging SET → canonical SET → staging
DEL) in withRetry. 3 attempts total, exponential backoff (1s, 2s, 4s)
≈ 7s worst-case before giving up. Each attempt re-stages with a fresh
runId so a previous attempt's staging key (if it landed server-side
but the response was lost) doesn't shadow the retry. The 5-min
staging TTL cleans up orphans naturally.

Also tag redisCommand errors so retry semantics are correct:
  - PERMANENT_4XX_STATUSES (400, 401, 403, 404, 410, 422, 451) get
    `nonRetryable: true` → withRetry exits the loop in ~10ms
  - 429 gets `retryAfterMs` from the Retry-After header → withRetry
    honors the upstream hint
  - 5xx and timeouts fall through with no flag → default exponential
    backoff applies

This is the second half of today's resilience work. PR #3649 caps
the digest replay-log lists at 100K entries to prevent the 500MB
Upstash overage that was likely back-pressuring these timeouts in
the first place. Together: #3649 removes the source of pressure;
this PR removes the single point of failure even when other
transients hit.

Tests: 5 new + 12 existing seed-utils-with-retry tests pass.
  - happy path (no retry, 3 fetch calls)
  - transient 503 → retry → success (4 calls, ≥1s elapsed)
  - permanent 401 → fast-fail, no retry (1 call, <500ms)
  - persistent 503 → exhausts 3 attempts → throws
  - 429 with Retry-After: 2 → wait ≥2s before retry

No behavior change for already-succeeding seeders. Resilience is
purely additive — the second/third attempt only fires when the
first attempt threw.

* review: correct backoff-math comment in atomicPublish (P2)

Greptile flagged: comment said "1s, 2s, 4s ≈ 7s worst case" but
withRetry(fn, maxRetries=2, 1000) only fires TWO backoff intervals
(before attempts 2 and 3), not three. Actual worst-case cumulative
backoff is 1s + 2s = ~3s, not ~7s.

Real worst-case before propagation includes per-attempt fetch time
(15s timeout each × 3 attempts = 45s) + ~3s cumulative backoff ≈ 48s.

No code change — comment-only fix.
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