diff --git a/hindsight-api-slim/hindsight_api/engine/retain/chunk_storage.py b/hindsight-api-slim/hindsight_api/engine/retain/chunk_storage.py index e1c5fab714..0f95fe2a69 100644 --- a/hindsight-api-slim/hindsight_api/engine/retain/chunk_storage.py +++ b/hindsight-api-slim/hindsight_api/engine/retain/chunk_storage.py @@ -64,8 +64,53 @@ async def delete_chunks_by_ids(conn, chunk_ids: list[str]) -> None: """ if not chunk_ids: return + + # PostgreSQL's FK cascade deletes child memory_links in executor-chosen + # order. Concurrent chunk deletes for the same bank can then lock overlapping + # memory_links in opposite orders and deadlock. Delete links explicitly in a + # total order before deleting chunks so every writer takes row locks the same + # way; the FK cascade still handles anything inserted later in this txn. + await conn.execute( + f""" + WITH target_units AS MATERIALIZED ( + SELECT id + FROM {fq_table("memory_units")} + WHERE chunk_id = ANY($1::text[]) + ), + ordered_links AS MATERIALIZED ( + SELECT ml.ctid + FROM {fq_table("memory_links")} ml + WHERE EXISTS ( + SELECT 1 + FROM target_units tu + WHERE tu.id = ml.from_unit_id OR tu.id = ml.to_unit_id + ) + ORDER BY + LEAST(ml.from_unit_id, ml.to_unit_id), + GREATEST(ml.from_unit_id, ml.to_unit_id), + ml.link_type, + COALESCE(ml.entity_id, '00000000-0000-0000-0000-000000000000'::uuid) + FOR UPDATE OF ml + ) + DELETE FROM {fq_table("memory_links")} ml + USING ordered_links ol + WHERE ml.ctid = ol.ctid + """, + chunk_ids, + ) await conn.execute( - f"DELETE FROM {fq_table('chunks')} WHERE chunk_id = ANY($1::text[])", + f""" + WITH ordered_chunks AS MATERIALIZED ( + SELECT chunk_id + FROM {fq_table("chunks")} + WHERE chunk_id = ANY($1::text[]) + ORDER BY chunk_id + FOR UPDATE + ) + DELETE FROM {fq_table("chunks")} c + USING ordered_chunks oc + WHERE c.chunk_id = oc.chunk_id + """, chunk_ids, ) diff --git a/hindsight-api-slim/tests/test_chunk_storage_delete_ordering.py b/hindsight-api-slim/tests/test_chunk_storage_delete_ordering.py new file mode 100644 index 0000000000..b25d535644 --- /dev/null +++ b/hindsight-api-slim/tests/test_chunk_storage_delete_ordering.py @@ -0,0 +1,50 @@ +"""Regression coverage for deterministic chunk deletion ordering.""" + +import pytest + +from hindsight_api.engine.retain import chunk_storage + + +class RecordingConn: + def __init__(self) -> None: + self.calls: list[tuple[str, tuple[object, ...]]] = [] + + async def execute(self, sql: str, *args: object) -> None: + self.calls.append((sql, args)) + + +@pytest.mark.asyncio +async def test_delete_chunks_by_ids_predeletes_links_before_chunks(): + conn = RecordingConn() + chunk_ids = ["chunk-b", "chunk-a"] + + await chunk_storage.delete_chunks_by_ids(conn, chunk_ids) + + assert len(conn.calls) == 2 + link_sql, link_args = conn.calls[0] + chunk_sql, chunk_args = conn.calls[1] + + assert link_args == (chunk_ids,) + assert chunk_args == (chunk_ids,) + + assert "DELETE FROM" in link_sql + assert "memory_links" in link_sql + assert "target_units AS MATERIALIZED" in link_sql + assert "ordered_links AS MATERIALIZED" in link_sql + assert "ORDER BY" in link_sql + assert "FOR UPDATE OF ml" in link_sql + + assert "DELETE FROM" in chunk_sql + assert "chunks" in chunk_sql + assert "ordered_chunks AS MATERIALIZED" in chunk_sql + assert "ORDER BY chunk_id" in chunk_sql + assert "FOR UPDATE" in chunk_sql + + +@pytest.mark.asyncio +async def test_delete_chunks_by_ids_noops_without_chunks(): + conn = RecordingConn() + + await chunk_storage.delete_chunks_by_ids(conn, []) + + assert conn.calls == []