Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion hindsight-api-slim/hindsight_api/engine/retain/chunk_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand Down
50 changes: 50 additions & 0 deletions hindsight-api-slim/tests/test_chunk_storage_delete_ordering.py
Original file line number Diff line number Diff line change
@@ -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 == []