perf+fix(graph-maintenance): catch the #2529 sweep deadlock in continuous perf, and drive dropped passes to zero

[nicoloboschi]

Summary

Supersedes / completes #2529. That PR fixed the entity_cooccurrences sweep deadlock by wrapping Pass 2/3 of run_graph_maintenance_job in retry_with_backoff. This PR includes @jordigilh's original fix commit unchanged, then answers three follow-ups:

1. Why did our continuous perf tests never catch this?
2. The retry wrap still dropped ~14% of sweep passes under load — can we get that to ~0?
3. Can we prevent the deadlock outright instead of retrying it?

1. Why continuous perf missed it — and a suite that catches it

The existing graph-maintenance perf suite runs run_graph_maintenance_job in isolation (populate → delete → one pass, zero concurrent writers). But the DeadlockDetectedError is a contention phenomenon: it only occurs when prune_stale_cooccurrences' unordered DELETE scan overlaps retain's sorted (entity_id_1, entity_id_2) cooccurrence upsert (entity_resolver._flush_pending). With no concurrent upsert there is no lock cycle, so the isolated suite structurally cannot reproduce it. And in production a losing deadlock silently drops a maintenance pass — visible only as slow graph bloat over time, never as a latency number. Coverage gap, not a tuning gap.

New suite graph-maintenance-contention (hindsight-dev/benchmarks/perf/system_perf.py) closes it: it seeds a hot set of stale cooccurrence pairs, then drives retain-shaped sorted upsert loops against loops calling run_graph_maintenance_job, and counts how many maintenance passes the resulting deadlock silently drops. The gate metric is the deadlock escape rate (dropped / max(observed, dropped)) — not a hard dropped == 0, so it tolerates a small retry tail while still cleanly separating a full regression.

2. Prevent the deadlock at the source (ordered locking)

Rather than only retrying, this PR removes the lock-order inversion that causes the cycle. prune_stale_cooccurrences (PostgreSQL) now selects its victim rows in the same sorted (entity_id_1, entity_id_2) order that retain's upsert locks them — a materialised FOR UPDATE CTE puts LockRows above the Sort, so locks are acquired ascending on both sides — then deletes the already-locked rows. Same order ⇒ no cycle. This mirrors how #2353 (queue) and entity_resolver already avoid their deadlocks (consistent lock ordering, not retries).

• PG only. Oracle's DELETE can't carry that ordered-lock CTE the same way, so it stays on the ORA-00060 retry path (documented in ops_oracle.py). Deliberate dialect asymmetry.

3. Retry hardening — the backstop

Ordered locking covers the dominant path (the 39/41 prod deadlocks). The retry_with_backoff wrap stays as the backstop for the residual paths (the prune_orphan_entities FK cascade, and Oracle), now hardened so it effectively never drops a pass on those either:

• Equal-jitter backoff in retry_with_backoff so concurrent retriers don't wake in lock-step and re-collide (benefits every caller). Covered by a new pure-function unit test.
• Larger retry budget for the idempotent background sweep (8 vs the default 3) — no client waits on it, so a longer jittered tail beats leaking stale graph rows.

Verified (perf suite, --scale small)

Config	Deadlocks (sweep)	Escape rate	Dropped	Duration	Suite
main (no fix)	41	100%	41	—	❌ FAIL
retry only	8–13	~14%	0–2	~30s	✅ PASS
retry hardened (jitter + budget 8)	8–18	0%	0	~30s	✅ PASS
+ ordered locking	0	0%	0	~10s	✅ PASS

With ordered locking the deadlock is prevented — 0 on both the sweep and upsert side — while 200 upserts + 336 sweeps genuinely ran concurrently, and it's ~3× faster (no backoff sleeps). Reverting all three fixes to main returns the suite to 100% escape / FAIL, so it still catches a full regression.

Note on the mechanism: "LockRows above Sort" is the documented/expected PG plan for a top-level ORDER BY … FOR UPDATE, not a hard contract — verified empirically (0 deadlocks across repeated runs), not assumed.

Tests

cd hindsight-api-slim
uv run pytest tests/test_graph_maintenance.py tests/test_graph_maintenance_deadlock.py tests/test_db_utils.py -q
# 20 passed  (incl. #2529's own deadlock tests, the CTE-ordered prune, and the new jitter unit test)

Perf suite (embedded pg0, no external setup):

cd hindsight-dev
uv run perf-test --suite graph-maintenance-contention --scale small     # PASS (0% escape, "prevented at source")

ruff / lint hooks clean on all changed files.

Notes for reviewers

• The contention suite is in the default SUITES set, so perf-test.yml's daily run gates on it going forward. If this lands before the fix on main, the daily perf job correctly goes red until it does.
• The suite's hollow-run guard now keys off whether the workloads ran (upserts + sweeps > 0) rather than off seeing deadlocks — otherwise a correct source-level fix (0 deadlocks) would be misread as "didn't reproduce."
• Credit to @jordigilh for the original deadlock diagnosis and fix (fix(graph-maintenance): retry cooccurrence sweep on deadlock #2529), preserved as the first commit here.