Skip to content

Retry the subject purge once on a Postgres deadlock - #389

Merged
smaramwbc merged 2 commits into
mainfrom
fix/purge-deadlock-retry
Sep 6, 2026
Merged

Retry the subject purge once on a Postgres deadlock#389
smaramwbc merged 2 commits into
mainfrom
fix/purge-deadlock-retry

Conversation

@smaramwbc

Copy link
Copy Markdown
Owner

Fixes the main-CI failure on run 33638444055: DELETE FROM memories deadlocked against a concurrent multi-row writer on the same subject (integration flake — first occurrence of this race; the entity upsert from #384 never ran in that job, litellm being absent, so this is a surfaced pre-existing race, not a #384 regression). The purge now rolls back and redoes its transaction once on SQLSTATE 40P01 — Postgres aborts exactly one deadlock participant, so the redo converges. Non-deadlock errors and a second deadlock still raise immediately.

3 new unit tests; 1052 unit + 288 integration passed; ruff clean. The failed run itself has been rerun to confirm flakiness.

Main CI run 33638444055 (09-02) failed with `DELETE FROM memories`
deadlocking in tests/integration/test_semantic.py: the purge's
multi-row DELETEs can cross lock order with another multi-row writer on
the same subject's rows (a still-draining compile batch, an embedding
backfill). Postgres aborts exactly one side (SQLSTATE 40P01), which the
endpoint surfaced as a raw 500.

A single rollback-and-redo converges — the competing transaction has
either finished or loses the rematch — so the purge now retries once on
a deadlock-shaped DBAPIError (matched via the wrapped driver error) and
logs `subject_delete_deadlock_retry`. Any other DB error, or a second
deadlock, still raises immediately.

Covered by unit tests: retry-then-succeed, give-up-after-second,
no-retry-on-non-deadlock.
@smaramwbc
smaramwbc force-pushed the fix/purge-deadlock-retry branch from bbbbb1d to 8a36c35 Compare September 6, 2026 12:10
The #384 concurrent-convergence test failed on this PR's CI run: the
exact-match and cosine-merge paths appended linked_memory_ids via ORM
read-modify-write, so two concurrent writers that both read the same
committed array each wrote back their own copy and silently dropped the
other's link. The unique index made concurrent writers converge on ONE
row, but not on one array.

Both paths now append through a single guarded SQL UPDATE (the same
ANY()/array_append CASE the ON CONFLICT branch uses): the UPDATE takes
the row lock and re-evaluates against the latest committed array, so
concurrent appends serialize, and the row is refreshed so callers see
the post-append state. Concurrency test now passes 12/12 consecutive
local runs; it failed nondeterministically before.
@smaramwbc

Copy link
Copy Markdown
Owner Author

The CI failure on the first push was the #384 concurrent-convergence test catching a REAL second race, not a flake: the exact-match and cosine-merge paths appended linked_memory_ids via ORM read-modify-write, so concurrent writers converged on one row but could silently drop each other's links. Second commit routes both paths through the same guarded ANY()/array_append UPDATE the ON CONFLICT branch uses — appends now serialize on the row lock. Concurrency test 12/12 locally; full suites green.

@smaramwbc
smaramwbc merged commit 543f078 into main Sep 6, 2026
6 checks passed
@smaramwbc
smaramwbc deleted the fix/purge-deadlock-retry branch September 6, 2026 12:17
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