Skip to content

fix(retention): batch every delete — the sweep had never once succeeded - #177

Merged
thejefflarson merged 1 commit into
mainfrom
fix/retention-batching
Aug 25, 2026
Merged

fix(retention): batch every delete — the sweep had never once succeeded#177
thejefflarson merged 1 commit into
mainfrom
fix/retention-batching

Conversation

@thejefflarson

Copy link
Copy Markdown
Owner

watcher has been serving 503 on /healthz:

{"db":true,"retention_last_success_age_secs":null,"retention_stalled":true,"status":"unhealthy"}

null, not stale — retention had never once completed in the life of the process. Two independent bugs, both in the history-table loop.

1. The deletes were unbatched

Each history table was pruned with a single DELETE FROM <t> WHERE <col> < cutoff. Against a backlog that exceeds the pool's 60s statement_timeout the statement is cancelled and rolls back entirely — the sweep deletes nothing, the backlog grows, the next attempt is slower. It cannot recover on its own.

Measured in production:

DELETE FROM logs WHERE time < now() - make_interval(days => $1)
  elapsed = 60.01s   rows_affected = 0
  retention sweep failed: canceling statement due to statement timeout

logs held 11,488,281 rows past its 7-day window — essentially the whole 15 GB table, with 5 indexes including a GIN on attributes.

prune_raw_metrics already solved exactly this with ctid-batching, and its own doc says "this is exactly how the table once reached 35 GB" (JEF-425). The module comment assumed per-table deletes were small enough to skip it. They aren't. All four tables now share prune_batched.

2. One failing table starved the rest

The loop used ?, so the first failure aborted the sweep. metric_series_rollups is ordered after logs — so once the logs delete began timing out hourly, the rollups table was never swept again. That's why it reached 20 GB / 14.8M rows.

Per-table errors are now collected. The sweep still fails as a whole (so /healthz keeps reporting the stall, correctly), but every other table gets pruned first.

Also: a seq scan every minute

selfmon runs max(bucket) on metric_series_rollups once a minute. Both existing indexes lead with name, so a bare max(bucket) can use neither:

plan time
before Parallel Seq Scan, 14.8M rows, 966k buffer reads 2907 ms
after Index Only Scan, 1 row 0.142 ms

Added to the online-DDL lane, not a migration — ADR 0021: CONCURRENTLY can't run inside sqlx::migrate!'s transaction. Already applied by hand in production while diagnosing; declared here so a fresh database gets it.

Tests

The existing retention_prunes_old_rows inserts two rows per table and asserts deleted >= 4it passes against the broken code, which is how this shipped. Two regression tests added that fail against the old shape:

  • retention_history_tables_drain_in_batchesbatch = 1 forces the loop; a prune that stops after one statement fails it
  • retention_one_failing_table_does_not_starve_the_rest — a BEFORE DELETE trigger fails spans (first in the list) and asserts logs (after it) is still pruned, and that the sweep still reports failure overall

Local gates green: cargo fmt --check, cargo clippy --all-targets -- -D warnings. Integration tests need Postgres and run on CI's sidecar — Docker isn't running on my machine, and I deliberately did not point DATABASE_URL at production, since pool_or_skip truncates every telemetry table.

Not fixed here

selfmon's pg_visibility_map_summary call fails with permission denied — the DB role lacks pg_stat_scan_tables. That's a role grant on the operator-managed cluster, not an app change.

🤖 Generated with Claude Code

https://claude.ai/code/session_013cYVqzH7Xfwea7fAozdQK7

watcher was serving 503 on /healthz with `retention_stalled: true` and
`retention_last_success_age_secs: null` — not stale, NEVER succeeded. Two
independent bugs, both in the history-table loop.

1. THE DELETES WERE UNBATCHED. Each history table was pruned with a single
   `DELETE FROM <t> WHERE <col> < cutoff`. Against a real backlog that exceeds
   the pool's 60s statement_timeout the statement is cancelled and ROLLS BACK
   ENTIRELY — so the sweep deletes nothing, the backlog grows, and the next
   attempt is slower. It cannot recover on its own.

   Measured in production 2026-08-25: `logs` held 11,488,281 rows past its
   7-day window (essentially the whole 15 GB table, 5 indexes including a GIN
   on `attributes`). The hourly sweep hit 60s with `rows_affected=0` every
   time, for the life of the process.

   `prune_raw_metrics` already solved exactly this with ctid-batching, and its
   own doc comment says "this is exactly how the table once reached 35 GB"
   (JEF-425). The module comment assumed per-TABLE deletes were small enough to
   skip it. They are not. All four tables now share `prune_batched`.

2. ONE FAILING TABLE STARVED THE REST. The loop used `?`, so the first failure
   aborted the sweep. `metric_series_rollups` is ordered AFTER `logs`, so once
   the `logs` delete began timing out hourly it was never swept again — which
   is why that table reached 20 GB and 14.8M rows. Per-table errors are now
   collected; the sweep still fails as a whole (so /healthz keeps reporting the
   stall, correctly) but every other table is pruned first.

Also adds `metric_series_rollups_bucket_idx (bucket DESC)` to the online-DDL
lane. `selfmon` runs `max(bucket)` every minute and both existing indexes lead
with `name`, so it planned a Parallel Seq Scan over 14.8M rows — 966k buffer
reads, ~2.9s per minute. With the index it is an Index Only Scan of one row:
0.142ms. (Applied by hand in production while diagnosing; declared here so a
fresh database gets it too. The lane, not a migration — ADR 0021: CONCURRENTLY
cannot run in sqlx::migrate!'s transaction.)

TESTS. The existing `retention_prunes_old_rows` inserts two rows per table and
asserts `deleted >= 4` — it passes with the broken shape, which is how this
shipped. Added two regression tests that fail against the old code:
  * `retention_history_tables_drain_in_batches` — batch=1 forces the loop, so a
    prune that stops after one statement fails it.
  * `retention_one_failing_table_does_not_starve_the_rest` — a BEFORE DELETE
    trigger fails `spans` (first in the list) and asserts `logs` (after it) is
    still pruned, and that the sweep as a whole still reports failure.

Not fixed here: `selfmon`'s hourly `pg_visibility_map_summary` call fails with
"permission denied" — the DB role lacks pg_stat_scan_tables. That is a role
grant on the operator-managed cluster, not an app change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013cYVqzH7Xfwea7fAozdQK7
@thejefflarson
thejefflarson force-pushed the fix/retention-batching branch from d1c1740 to b2b3d31 Compare August 25, 2026 03:26
@thejefflarson
thejefflarson merged commit 7539cd3 into main Aug 25, 2026
5 checks passed
@thejefflarson
thejefflarson deleted the fix/retention-batching branch August 25, 2026 03:29
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