fix(reliability): restore OOM, stuck-row recovery, backup-OK log, propagation lease#38
Merged
Merged
Conversation
…nner D26-F4 — Replace io.ReadAll(s3Reader) with io.TeeReader → temp-file + sha256.New() hasher pattern. Multi-GB pro/team backups previously OOM-killed the worker pod because the SHA-256 integrity gate must precede pg_restore (its --clean --if-exists is destructive — cannot stream-verify in-band). Memory footprint drops from O(backup-size) to O(io.Copy buffer ≈ 32 KiB). Temp file lives under os.TempDir(), removed via defer. D26-F5 — Add recoverStuckRestores mirroring customer_backup_runner's recoverStuckRows pass. A pod kill mid-pg_restore previously orphaned the resource_restores row at status='running' forever (the pending-row sweep only selects status='pending'). Stuck rows are now marked 'failed' with error_summary='worker_killed_during_restore: ...' once started_at exceeds restorePerRunTimeout. Deliberately NOT re-queued (unlike the backup path) — pg_restore is destructive, silent retry of a half-completed restore could compound partial-state damage; customer must manually re-issue. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
D26-F7 — infra/newrelic/alerts/backup-stale-36h.json keys on the literal
string 'backup OK' (predicate: message LIKE '%backup OK%'). The k8s
CronJob-based backups (postgres-customers-backup, mongodb-backup,
redis-provision-backup) emit '[<ts>] backup OK' per shell convention; the
River-based platform_db_backup job previously emitted only the structured
'jobs.platform_db_backup.succeeded' slog line and was invisible to the
36h/60h staleness alert.
Add a second slog.Info('backup OK', kind='platform_db', ...) line
alongside the existing structured emit. Both surfaces matter: the
structured line stays for dashboard/query, the literal line triggers the
alert. Rule 25 — every alert needs its trigger to actually fire.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
D22-P3 — pickEligible's tx committed after the SELECT FOR UPDATE SKIP LOCKED, leaving next_attempt_at unchanged. A pod crash between commit and the handler's terminal markApplied/markRetry left the row eligible for re-pick on the next 30s tick with attempts unchanged — a flaky pod could spin the same row through dispatch every 30s without ever incurring backoff, eventually exhausting maxAttempts against transient pod failures rather than genuine downstream outages. Add a propagationLeaseDuration (5m, ≥ propagationDispatchTimeout) push on next_attempt_at INSIDE the same picker tx. Healthy run: markApplied or markRetry overwrites the lease as before. Crash run: the row falls back to the lease window — re-picked after 5m, attempts unchanged (lease is a crash-recovery mechanism, not a dispatch failure → should not consume a retry attempt). Hand-rolled pgUUIDArray formatter so the worker stays driver-agnostic (no lib/pq pq.Array import). Lease bump is best-effort — a bump failure falls back to pre-fix behavior + logs WARN; the dispatch loop remains correct. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mastermanas805
added a commit
that referenced
this pull request
May 21, 2026
…pagation lease (#38) * fix(restore): stream-hash + stuck-row recovery in customer_restore_runner D26-F4 — Replace io.ReadAll(s3Reader) with io.TeeReader → temp-file + sha256.New() hasher pattern. Multi-GB pro/team backups previously OOM-killed the worker pod because the SHA-256 integrity gate must precede pg_restore (its --clean --if-exists is destructive — cannot stream-verify in-band). Memory footprint drops from O(backup-size) to O(io.Copy buffer ≈ 32 KiB). Temp file lives under os.TempDir(), removed via defer. D26-F5 — Add recoverStuckRestores mirroring customer_backup_runner's recoverStuckRows pass. A pod kill mid-pg_restore previously orphaned the resource_restores row at status='running' forever (the pending-row sweep only selects status='pending'). Stuck rows are now marked 'failed' with error_summary='worker_killed_during_restore: ...' once started_at exceeds restorePerRunTimeout. Deliberately NOT re-queued (unlike the backup path) — pg_restore is destructive, silent retry of a half-completed restore could compound partial-state damage; customer must manually re-issue. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(platform-backup): emit 'backup OK' log line for NR staleness alert D26-F7 — infra/newrelic/alerts/backup-stale-36h.json keys on the literal string 'backup OK' (predicate: message LIKE '%backup OK%'). The k8s CronJob-based backups (postgres-customers-backup, mongodb-backup, redis-provision-backup) emit '[<ts>] backup OK' per shell convention; the River-based platform_db_backup job previously emitted only the structured 'jobs.platform_db_backup.succeeded' slog line and was invisible to the 36h/60h staleness alert. Add a second slog.Info('backup OK', kind='platform_db', ...) line alongside the existing structured emit. Both surfaces matter: the structured line stays for dashboard/query, the literal line triggers the alert. Rule 25 — every alert needs its trigger to actually fire. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(propagation): crash-safe lease bump in pickEligible tx D22-P3 — pickEligible's tx committed after the SELECT FOR UPDATE SKIP LOCKED, leaving next_attempt_at unchanged. A pod crash between commit and the handler's terminal markApplied/markRetry left the row eligible for re-pick on the next 30s tick with attempts unchanged — a flaky pod could spin the same row through dispatch every 30s without ever incurring backoff, eventually exhausting maxAttempts against transient pod failures rather than genuine downstream outages. Add a propagationLeaseDuration (5m, ≥ propagationDispatchTimeout) push on next_attempt_at INSIDE the same picker tx. Healthy run: markApplied or markRetry overwrites the lease as before. Crash run: the row falls back to the lease window — re-picked after 5m, attempts unchanged (lease is a crash-recovery mechanism, not a dispatch failure → should not consume a retry attempt). Hand-rolled pgUUIDArray formatter so the worker stays driver-agnostic (no lib/pq pq.Array import). Lease bump is best-effort — a bump failure falls back to pre-fix behavior + logs WARN; the dispatch loop remains correct. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reliability cluster sweep — four fixes from the 2026-05-21 bug-hunt (D22 + D26):
customer_restore_runnerOOM on multi-GB pro/team backups → streamingio.TeeReader+ temp file (noio.ReadAll)customer_restore_runnerno stuck-row recovery →recoverStuckRestores(mirrors backup runner pattern; marks failed, does NOT re-queue because pg_restore is destructive)platform_db_backupinvisible to NR staleness alert → literalslog.Info("backup OK", ...)line alongside the existing structured emit (alert keys onmessage LIKE '%backup OK%')propagation_runnercrash between pick and markRetry let row re-pick every 30s with no backoff → 5m lease bump onnext_attempt_atinside the picker tx (overwritten by happy-pathmarkApplied/markRetry)Test plan
make gategreen (build + vet +go test ./... -short -count=1)len(out) > 0guard)TestPropagation_UnknownKindIntegration_BoundedRetries) updated/healthzcommit_id after deploy (operator step)Notes
failed(not re-queued) because pg_restore is destructive and silent retry of a half-completed restore could compound partial-state damage. Customer must manually re-issue.pgUUIDArrayhand-rolled to keep worker driver-agnostic (nolib/pqimport in propagation path).Per CLAUDE.md rules 16/17/22 — D22/D26 enumeration was a 4-site fix and all 4 sites are addressed.
🤖 Generated with Claude Code