Skip to content

Production hardening: fix duplicate-run race, scale cliffs, crash/leak/error-path bugs - #5

Merged
NitinKumar004 merged 8 commits into
mainfrom
hardening/prod-audit-fixes
Aug 9, 2026
Merged

Production hardening: fix duplicate-run race, scale cliffs, crash/leak/error-path bugs#5
NitinKumar004 merged 8 commits into
mainfrom
hardening/prod-audit-fixes

Conversation

@NitinKumar004

Copy link
Copy Markdown
Contributor

Production-readiness audit + fixes

Acting as SDET: I set up a sandbox, tested it like a real user at scale (millions/billions of rows), ran adversarial code audits of the concurrency / scale / error-path subsystems, and fixed everything through Medium severity in this PR. Every fix has a test or a live-reproduced before/after.

Stacked on feature/adaptive-throttle (base), so this diff is only the audit fixes.

🔴 Critical

  • Duplicate concurrent runs per task. ActiveForTaskINSERT wasn't atomic and had no DB constraint — I reproduced 2 active runs from 10 concurrent starts, i.e. two runners backfilling the same table at once. Fixed with a partial unique index (runs_one_active_per_task); StartRun maps the conflict to a clean 409. New test fires 12 concurrent creates → exactly 1 wins.

🟠 High

  • Fleet lease planning OOM / infinite loop on huge/sparse key spaces (snowflake ids to 10^18, values near MaxInt64). PlanLeases is now capped at MaxLeases, stride-widening, and overflow-safe; Enqueue ships in chunks. Unit-tested with 10^12 span and MaxInt64 edges.
  • Fleet run stuck running forever on a persistently failing target. Added a stall detector (no progress + climbing lease failures) — live test: a failing fleet run now fails in ~20s instead of hanging.
  • Unrecovered panics crashed the whole process. Runner and worker goroutines now recover() and fail just that run.
  • Websocket subscription leak on abrupt tab close — the hub now reaps dead subscribers.
  • Fleet false-reap: a batch slower than the reap cutoff made the monitor hand a live worker's lease to another (duplicate processing). Heartbeats now run on a background ticker, independent of batch duration.
  • No statement timeout / unbounded connection pool on the target — added pool limits + per-batch and count-query timeouts.
  • NULL cursor rows silently skipped — now surfaced via a coverage_gap event. (Non-unique cursors are actually safe — verified.)

🟡 Medium

  • GET /runs/{missing} and POST /tasks/{missing}/runs now 404 (were 500).
  • Duplicate task name and start-while-active now 409 (were 500 leaking the Postgres constraint).
  • HTTP callback: a 2xx with empty/malformed body now quarantines the batch (retryable) instead of failing the run or silently claiming all rows applied. Live-verified: 1500 rows quarantined, run still succeeds.
  • /app auth bypass anchored to a path boundary (/apple, /app/../tasks no longer skip auth).
  • RetryQuarantine hardcoded the pgx driver → broke MySQL quarantine retry. Now dialect-aware.
  • Dashboard: websocket auto-reconnect with backoff (shows "Reconnecting…" instead of freezing), slider debounce cleared on unmount, invalid run-id guarded.

🟢 Efficiency (billion-row scale)

  • Removed the per-batch runs.Get in solo mode (in-memory counters) — halves control-store reads on long runs.

Verified

  • Full unit + integration suite green (Postgres + MySQL + Redis), including new tests: OneActiveRunPerTask (12-way race), PlanLeasesBounded, PlanLeasesOverflowSafe, ActiveRunFreesOnTerminal.
  • Live re-tests of every original breakage: dup-run→1 active, 404s, 409s, HTTP-callback quarantine, fleet-fail-in-20s, and a clean 1M-row backfill still exact.

Deliberately deferred (documented, low-risk)

  • Cross-host clock skew on the 10s reap cutoff, and multi-replica fleet-monitor double-reap — both require a specific multi-node topology and are noted for a follow-up; single-monitor + background heartbeats cover the common case.
  • Unbounded checkpoints growth (1 row/batch) — a retention/prune job is a separate follow-up.

Closes the check-then-insert race where two concurrent starts (manual +
scheduler, or double-click) could both create an active run and
double-process the whole table. A partial unique index is the atomic
backstop; StartRun maps the conflict to ErrTaskBusy.
Missing run/task now 404; duplicate task name and start-while-active now
409 instead of a 500 that leaked the Postgres constraint.
Cap MaxOpenConns/idle/lifetime so a fleet can't exhaust the target's
connections and stale conns are recycled on long runs; add per-batch and
count-query timeouts so a hung query can't pin a run in 'running' forever.
Also drop the per-batch runs.Get in solo mode (in-memory counters).
PlanLeases caps at MaxLeases and widens the stride for huge/sparse key
spaces (e.g. snowflake ids to 10^18), and is safe near MaxInt64 — no more
OOM or infinite loop. Enqueue ships leases in chunks.
A persistently failing target now fails the run via a stall detector
(no progress + climbing lease failures) instead of churning forever.
Heartbeats run on a background ticker so a slow/throttled batch can't be
falsely reaped. Runner/worker goroutines recover from panics so one bad
run can't crash the process.
…ess/failure

A 2xx with an empty or malformed body no longer either fails the whole run
or silently marks every row affected — the batch's rows are quarantined for
retry so the operator sees the contract violation.
…rage gap

Reap dead websocket subscribers (abrupt tab close no longer leaks). Anchor
the /app auth bypass to a path boundary. Emit a coverage_gap event when a
run walks fewer rows than counted (NULLable cursor).
Live console reconnects with backoff (shows Reconnecting… instead of
silently freezing), clears the slider debounce on unmount, and guards an
invalid run id.
Base automatically changed from feature/adaptive-throttle to main August 9, 2026 18:36
@NitinKumar004
NitinKumar004 merged commit ea1860d into main Aug 9, 2026
3 checks passed
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