Skip to main content
← Back to list
01Issue
BugOpenSwamp Club
AssigneesNone

Relationships

#1254 Telemetry stats consumer: insert_id dedup commits before the write it guards — any upsert failure is silently swallowed on retry

Opened by keeb · 7/18/2026

Split out of #1249 (see its 'Related but separate concern' section), where the mechanism was confirmed in production and reproduced locally.

Problem

In StatsConsumer._publish (services/telemetry/lib/consumers/stats.ts), the insert_id dedup writes to processed_inserts and COMMITS before the user_metrics upsert runs. When the upsert throws — for any reason — the batch is marked failed and retried, but on retry the dedup step sees every insert_id as already processed, the fresh set is empty, and the function returns early with outcome 'duplicate'. The consumer reports success, the batch is marked delivered and drained, and the data is gone.

Confirmed behavior (from the #1249 investigation and local reproduction on an isolated stack): one alert fires on the first failure, then the retry 'succeeds' silently. The batch never reaches the attempts >= 5 dead-letter parking in retryFailed — this ordering routes around the dead-letter design entirely, which explicitly reasons about not losing data during a consumer outage.

#1249 fixed the one known trigger (unsanitized command/subcommand producing invalid Mongo update paths), but the window is generic: a transient Mongo error, a timeout, or an OOM between the dedup commit and the upsert commit behaves identically — permanent, unreported loss of that batch's per-user metrics.

Known residual trigger (ADV-2 from the #1249 review)

If any user_metrics docs were already corrupted by the pre-#1249 quiet variant (a dotted subcommand like 'foo.bar' writing a nested object where a flat counter belongs — unconfirmed in production), a post-fix flat $inc on that same path fails with 'Cannot apply $inc to a value of non-numeric type'. The writeError handler in upsertPerDistinctId only migrate-retries 'Cannot create field' errors and rethrows everything else, so such a batch would fail once and then be swallowed by exactly this ordering flaw.

Direction

Options include moving the dedup commit after the upsert (or making it two-phase: reserve → confirm), scoping the dedup marks per-consumer-outcome, or having the retry path bypass the dedup for events whose consumer_status is not delivered. Whatever the shape, the invariant should be: a batch that never committed its writes must never be reported as delivered.

Affected code

  • services/telemetry/lib/consumers/stats.ts — _publish dedup-before-upsert ordering
  • services/telemetry/lib/watcher.ts — retryFailed dead-letter parking that the ordering bypasses
02Bog Flow
◉OPEN○TRIAGED○IN PROGRESS○SHIPPED

Open

7/18/2026, 3:31:06 PM

No activity in this phase yet.

03Sludge Pulse

Sign in to post a ripple.