Skip to content

#2143: retry drop_chunks once on deadlock before the DELETE fallback - #2145

Merged
erikdarlingdata merged 2 commits into
devfrom
retention-40p01-retry-2143
Aug 9, 2026
Merged

#2143: retry drop_chunks once on deadlock before the DELETE fallback#2145
erikdarlingdata merged 2 commits into
devfrom
retention-40p01-retry-2143

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

Option 3 from the issue (product-side retry), which the purge e2e then inherits for free.

The field case (nightly run 31334656715): drop_chunks on collection_log lost a 40P01 deadlock to a background job whose chunk locks clear within milliseconds of the abort — then the row-by-row DELETE fallback deadlocked against the same partner, so one transient loss burned the entire purge cycle and failed the e2e's survival assertion.

The fix: 40P01 earns exactly one immediate retry. A second deadlock in a row is standing contention, where the existing fallback-plus-next-cycle posture is right and a retry loop camped on a lock queue is not. Non-deadlock failures keep the original single-shot behavior; cancellation still propagates.

Pins (the retry logic is seamed through an internal delegate, so all arms test without a store): one deadlock → retries once and succeeds; two deadlocks → exactly two attempts then null (fallback); non-deadlock → one attempt, no retry; clean run → single-shot.

Closes #2143 on merge (I'll close explicitly — dev-target PRs don't auto-close).

erikdarlingdata and others added 2 commits August 10, 2026 01:10
The nightly's purge e2e caught the field case: drop_chunks lost a deadlock
to a background job whose chunk locks clear in milliseconds, then the DELETE
fallback deadlocked against the same partner — one transient loss burned the
whole purge cycle. 40P01 now earns exactly one immediate retry, seamed
through an internal delegate so all four arms pin without a store: one
deadlock retries and succeeds, two in a row give up to the fallback
(standing contention), non-deadlock failures never retry, clean runs stay
single-shot.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(Split from the parent commit: the byte-level insert refused its non-unique
anchor and the commit chain did not gate on it — the anchor discipline
worked, the shell plumbing did not.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 9, 2026

Copy link
Copy Markdown

Reviewed. This is a tightly scoped fix and looks correct:

  • Retry logic: bounded to exactly one retry on 40P01 (deadlock), verified by the attempt == 1 guard in the exception filter — a second deadlock falls through to the general catch and returns null (DELETE fallback), matching the documented intent. Non-deadlock exceptions keep single-shot behavior. OperationCanceledException is excluded from both catch clauses, so cancellation still propagates as claimed.
  • No stale-connection risk on retry: DropChunksOneAsync's delegate re-opens a fresh NpgsqlConnection on each invocation, so the retry doesn't reuse a connection left in an aborted-transaction state from the first attempt.
  • Test coverage is solid and matches the four documented arms (one deadlock → retry+succeed, two deadlocks → fallback, non-deadlock → no retry, clean run → single-shot), and follows the existing PostgresException construction pattern used elsewhere in the test suite.
  • Lite/Darling parity: no drift — Lite has no Postgres/TimescaleDB dependency (DuckDB + its own parquet RetentionService), so there's no counterpart code path for this fix to apply to.
  • Security/perf: tableName values passed through are compile-time constants, not user input (consistent with the existing interpolation-is-safe rationale in this file); the extra retry attempt only fires on a rare 40P01 and processing is sequential per table, so there's no meaningful perf regression.

No findings to flag.

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