fix(search): bound get_edge_invalidation_candidates before collect (Neo4j)#1671
fix(search): bound get_edge_invalidation_candidates before collect (Neo4j)#1671mvalee wants to merge 1 commit into
Conversation
…eo4j)
get_edge_invalidation_candidates matches every RELATES_TO edge touching either
endpoint of a candidate edge, collects them all - each row carrying
fact_embedding, plus properties(e) which repeats the same embedding - and only
then slices to RELEVANT_SCHEMA_LIMIT (10).
On a graph with high-degree entities that materialises an entire neighbourhood to
return ten rows. Measured on Neo4j 5.26 Community, ~32.6k Episodic / ~22.8k Entity
/ ~101k RELATES_TO with 1024-dim embeddings, sampling SHOW TRANSACTIONS at 150ms:
a single call peaked at 712 MiB against a 716.8 MiB transaction pool, and still
exhausted the pool after it was raised to 1.4 GiB. Because the collect is
unbounded, no pool size fixes it - cost tracks the degree of the busiest entity,
which grows monotonically as a graph fills.
The consequence is worse than a slow query. The resulting TransientError reaches
`await job()` in AsyncWorker.worker(), which catches only asyncio.CancelledError,
so the ingestion worker dies and is never respawned while /messages keeps
returning 202 and /healthcheck keeps returning OK.
Neo4j's CALL subquery applies ORDER BY + LIMIT per `edge` row, so at most $limit
rows per candidate ever materialise.
Verified against a production graph, same inputs, both queries:
- results identical as sets across 40 sampled candidate edges (same keys, same
match counts, same uuids). Two differed only in ordering, which is the
equal-score tie case and is already non-deterministic in the current query.
- PROFILE on edges attached to the highest-degree entity:
before 571,073,600 bytes (544.6 MiB), 7,416 ms
after 588,488 bytes ( 0.56 MiB), 5,303 ms
Scoped to GraphProvider.NEO4J. The default branch also serves FalkorDB, whose
CALL-subquery support I could not verify, so it is left untouched. Kuzu and
Neptune are likewise unchanged.
Note for maintainers: the Kuzu branch already applies LIMIT before collect, but
after UNWIND without a per-row subquery, so it appears to cap total matches rather
than matches per candidate edge. Left alone here as a separate concern.
The same collect-then-slice shape appears in get_relevant_edges; that path is
pair-constrained so it is far cheaper (8 MiB worst case measured here) and is not
changed in this PR.
|
I have read the CLA Document and I hereby sign the CLA behalf on myself, e-mail: example@example.com or I have read the CLA Document and I hereby sign the CLA behalf of my company, e-mail: example@example.com Signature is valid for 6 months. This bot will be retriggered when the Contributor License Agreement comment has been provided. Posted by the CLA Assistant Lite bot. |
|
I have read the CLA Document and I hereby sign the CLA behalf on myself, e-mail: michiel@valee.be |
Problem
get_edge_invalidation_candidatesmatches everyRELATES_TOedge touching either endpoint of a candidate edge, collects them all, and only then slices toRELEVANT_SCHEMA_LIMIT(10). Each collected row carriesfact_embedding, plusattributes: properties(e)which repeats the same embedding.On a graph with high-degree entities this materialises an entire neighbourhood in order to return ten rows.
Measurements
Neo4j 5.26 Community, ~32.6k
Episodic/ ~22.8kEntity/ ~101kRELATES_TO, 1024-dim embeddings. SamplingSHOW TRANSACTIONSat 150ms intervals:db.memory.transaction.total.maxBoth figures are ceiling-bounded rather than true demand, which is the point: the collect is unbounded, so no pool size fixes it. Cost tracks the degree of the busiest entity, which grows monotonically as a graph fills. In our deployment the top entity had a
RELATES_TOdegree of 15,535.Why this is more than a slow query
The resulting
TransientErrorreachesawait job()inAsyncWorker.worker(), which catches onlyasyncio.CancelledError. The ingestion worker dies and is never respawned, whilePOST /messageskeeps returning202and/healthcheckkeeps returning OK. Ingestion stops with every health surface green.Fix
Neo4j's
CALLsubquery appliesORDER BY+LIMITperedgerow, so at most$limitrows per candidate ever materialise. Same rows, same order.Verification
Both queries run against a production graph with identical inputs:
Correctness - across 40 sampled candidate edges, results are identical as sets (same keys, same match counts, same uuids). Two differed only in ordering, which is the equal-score tie case and is already non-deterministic in the current query.
Memory -
PROFILEon edges attached to the highest-degree entity:Roughly 970x less memory, and faster.
Scope
Gated to
GraphProvider.NEO4J. The default branch also serves FalkorDB, whoseCALLsubquery support I could not verify, so it is left untouched. Kuzu and Neptune are unchanged.Two related things I noticed but deliberately did not change, happy to follow up if useful:
LIMITbeforecollect, but afterUNWINDwithout a per-row subquery, so it appears to cap total matches rather than matches per candidate edge.get_relevant_edgeshas the same collect-then-slice shape. It is pair-constrained so far cheaper (8 MiB worst case measured here), and keeping it out keeps this PR reviewable.Related: #723 and #763 (the worker-death amplification described above is what makes these token/limit issues expensive in practice).