perf(graph): decouple stale-cooccurrence prune from hub-entity degree (#3367) - #3408
Merged
Merged
Conversation
…#3367) `prune_stale_cooccurrences` (PG) checked staleness with a correlated `NOT EXISTS (… INTERSECT …)` evaluated once per cooccurrence row. Each evaluation re-scanned a hub entity's full membership set, so cost scaled as (cooccurrence rows) × (hub degree) — 88-140s on a real bank with a ~22K-degree hub (215s in a 12K-degree repro). #2473 had swapped an earlier hub-rescanning self-join to that INTERSECT, but only made each per-row check cheaper; it kept the per-row structure, so the product resurfaced at scale. Decide staleness against a SET of currently-live pairs built ONCE per sweep: a `WITH live AS MATERIALIZED` unit-grouped self-join emits every co-occurring (e1<e2) pair, its cost driven by unit degree (small) not entity degree, and the victims anti-join hashes against it. `live` is scoped to the bank's entities so a per-bank sweep stays O(bank), not O(schema), across a multi-bank maintenance cycle. The #2529 ordered-lock (`ORDER BY … FOR UPDATE OF c`) is preserved. Oracle already used a set-based form and is unchanged. Repro (12K-degree hub, 14K cooccurrence rows): 215,329ms -> ~250ms, identical delete set. Added a regression test covering partial pruning around a hub and bank-scoping isolation.
nicoloboschi
added a commit
that referenced
this pull request
Aug 12, 2026
…ased staleness check Rebase reconciliation with #3408, which landed the same statement while this was in review. #3408 kept the cooccurrence prune bank-wide and made it cheap by deciding staleness against a set of live pairs built once, instead of a correlated INTERSECT re-scanned per row — removing the (rows judged) x (hub degree) product. That is the better predicate, and it composes with the queue scoping rather than competing with it: `live` is now seeded from the *claimed candidates'* units instead of the whole bank's. Correctness holds because every pair being judged has a candidate as an endpoint, so any unit still grounding one of those pairs references a candidate and is in the seeded set. Measured on the dense fixture (100k entities, 1.5M unit_entities, 2.86M cooccurrences): bank-wide, set-based (#3408 as merged) did not finish in 10 min batch of 50, per-pair INTERSECT (mine) 2.0 s batch of 50, composed 16-65 ms The batch-size rationale is updated to the new numbers; 50 still holds, now with three orders of magnitude of margin instead of one. #3367's hub/bank- scoping regression test is kept, adapted to seed candidates. Also re-chains the migration onto d9c1a7b4e2f6, which took the same parent on main and would otherwise leave two alembic heads.
nicoloboschi
added a commit
that referenced
this pull request
Aug 12, 2026
…ased staleness check Rebase reconciliation with #3408, which landed the same statement while this was in review. staleness against a set of live pairs built once, instead of a correlated INTERSECT re-scanned per row — removing the (rows judged) x (hub degree) product. That is the better predicate, and it composes with the queue scoping rather than competing with it: `live` is now seeded from the *claimed candidates'* units instead of the whole bank's. Correctness holds because every pair being judged has a candidate as an endpoint, so any unit still grounding one of those pairs references a candidate and is in the seeded set. Measured on the dense fixture (100k entities, 1.5M unit_entities, 2.86M cooccurrences): bank-wide, set-based (#3408 as merged) did not finish in 10 min batch of 50, per-pair INTERSECT (mine) 2.0 s batch of 50, composed 16-65 ms The batch-size rationale is updated to the new numbers; 50 still holds, now with three orders of magnitude of margin instead of one. #3367's hub/bank- scoping regression test is kept, adapted to seed candidates. Also re-chains the migration onto d9c1a7b4e2f6, which took the same parent on main and would otherwise leave two alembic heads.
nicoloboschi
added a commit
that referenced
this pull request
Aug 12, 2026
…me enqueue Drops the "queue every entity at creation" change and keeps the rest of the review round (dataclass pass results, the Oracle IN-list chunking on the by-unit enqueue, the per-site enqueue tests, the migration-seed test, the budget's follow-up-chain test, the stale-comment sweep). The birth-time enqueue existed to reclaim an entity created in retain's Phase 1 whose Phase-2 link never landed. It is not worth what it costs: such a row is a single entry in the registry with no postings and no cooccurrences, and #2662 exists because the retry is *supposed* to adopt it — Phase 2 reasserts resolved parents under FOR KEY SHARE precisely so a pruner cannot delete one out from under it. Pointing the pruner at every freshly created entity leans on that race for a leak that is one row wide. #3408 landing the set-based predicate is what made the trade obviously bad: the expensive half of this job was never those rows. Entities created but never linked are therefore no longer proactively reclaimed. The migration's one-time seed still clears the population a bank has already accumulated.
nicoloboschi
added a commit
that referenced
this pull request
Aug 12, 2026
…k-wide sweep (#3222) (#3409) * fix(graph-maintenance): drive the entity prune off a queue, not a bank-wide sweep (#3222) The graph_maintenance job's Pass 2/3 were two bank-wide statements re-evaluated on every invocation, whether or not anything had changed: the orphan-entity prune probed once per entity in the bank, and the stale-cooccurrence prune evaluated an INTERSECT per cooccurrence row in the bank. Their cost tracked the size of the bank rather than the size of the delete, so past a few million rows neither could finish inside asyncpg's 60s command timeout. The job then failed on every run with a bare TimeoutError, forever, on exactly the banks that most needed it — and, because db_utils treats a timeout as transient, re-ran the doomed statement nine times per attempt, holding a worker slot for ~10 minutes each time. Both prunes are now driven by `entity_maintenance_queue`, filled inside the deleting transaction the way `graph_maintenance_queue` already is for the relink pass. A run claims a bounded batch of candidate entities, prunes what is genuinely dead, and commits — so the cost is O(delta), and the work already done survives whatever stops the run. Measured on a dense fixture (100k entities, 1.5M unit_entities, 2.86M cooccurrences, hub entities holding 150-400 postings): bank-wide orphan prune 9.6s → batch of 50: 15ms bank-wide cooccurrence prune >11 min → batch of 50: 2.0s (cancelled; ~1.7ms per pair over 2.86M pairs) Also: * A wall-clock budget for the whole job. Both passes commit per batch, so exhausting it is not a failure — the run reports `queues_drained: false`, logs it, and chains a follow-up (under a real queue; a synchronous backend would recurse instead of schedule). Large backlogs converge over runs. * The scoping predicate is a UNION of the two endpoint columns, not `entity_id_1 = ANY(...) OR entity_id_2 = ANY(...)` — that OR is the #3387 shape and cannot be driven from either index. * Every site that removes units or replaces entity postings now enqueues candidates: document delete, single and bulk memory delete, curation edit/invalidate, document re-ingest, and the delta-retain chunk cascade. * The migration seeds the queue with every existing entity, so garbage a bank accumulated while its sweep was failing is still reclaimed — incrementally, a bounded batch per run, instead of in one statement that cannot finish. * fix(graph-maintenance): compose the queue-scoped prune with the set-based staleness check Rebase reconciliation with #3408, which landed the same statement while this was in review. staleness against a set of live pairs built once, instead of a correlated INTERSECT re-scanned per row — removing the (rows judged) x (hub degree) product. That is the better predicate, and it composes with the queue scoping rather than competing with it: `live` is now seeded from the *claimed candidates'* units instead of the whole bank's. Correctness holds because every pair being judged has a candidate as an endpoint, so any unit still grounding one of those pairs references a candidate and is in the seeded set. Measured on the dense fixture (100k entities, 1.5M unit_entities, 2.86M cooccurrences): bank-wide, set-based (#3408 as merged) did not finish in 10 min batch of 50, per-pair INTERSECT (mine) 2.0 s batch of 50, composed 16-65 ms The batch-size rationale is updated to the new numbers; 50 still holds, now with three orders of magnitude of margin instead of one. #3367's hub/bank- scoping regression test is kept, adapted to seed candidates. Also re-chains the migration onto d9c1a7b4e2f6, which took the same parent on main and would otherwise leave two alembic heads. * fix(graph-maintenance): restore the review fixes without the birth-time enqueue Drops the "queue every entity at creation" change and keeps the rest of the review round (dataclass pass results, the Oracle IN-list chunking on the by-unit enqueue, the per-site enqueue tests, the migration-seed test, the budget's follow-up-chain test, the stale-comment sweep). The birth-time enqueue existed to reclaim an entity created in retain's Phase 1 whose Phase-2 link never landed. It is not worth what it costs: such a row is a single entry in the registry with no postings and no cooccurrences, and #2662 exists because the retry is *supposed* to adopt it — Phase 2 reasserts resolved parents under FOR KEY SHARE precisely so a pruner cannot delete one out from under it. Pointing the pruner at every freshly created entity leans on that race for a leak that is one row wide. #3408 landing the set-based predicate is what made the trade obviously bad: the expensive half of this job was never those rows. Entities created but never linked are therefore no longer proactively reclaimed. The migration's one-time seed still clears the population a bank has already accumulated. * fix(graph-maintenance): don't backfill the entity queue on upgrade The migration seeded one queue row per existing entity so a bank could reclaim what it stranded while its bank-wide sweep was failing. That is the wrong trade: the INSERT runs inside a migration at API startup, so a large deployment pays a slow upgrade writing a row per entity, and then a prune check for every one of them — a self-inflicted backlog to collect rows that cost the bank nothing. The queue now starts empty and fills from real deletes. Historical strays stay until something touches them; they are single registry rows with no postings and no cooccurrences. The migration test pins the two properties that are easy to lose later: the upgrade enqueues nothing, and the composite key collapses overlapping deletes into one row (which is also what the #3034 locking upsert conflicts on).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3367 —
prune_stale_cooccurrences(graph maintenance's stale-cooccurrence sweep) scaled with hub-entity degree, hitting 88–140s on a real bank with a ~22K-degree hub.The PG path decided staleness with a correlated
NOT EXISTS (… INTERSECT …)evaluated once per cooccurrence row, and each evaluation re-scanned a hub entity's full membership set — cost(cooccurrence rows) × (hub degree). #2473 had swapped an earlier hub-rescanning self-join to thatINTERSECT, but only made each per-row check cheaper; it kept the per-row structure, so the product resurfaced once the table/hub grew again.Fix
Decide staleness against a set of currently-live pairs built once per sweep:
liveis scoped to the bank's entities, so a per-bank sweep staysO(bank), notO(schema), across a multi-bank maintenance cycle (graph maintenance runs per bank).MATERIALIZEDkeeps the planner from inliningliveback into a per-row correlated plan.ORDER BY entity_id_1, entity_id_2 FOR UPDATE OF c) is preserved, so the deadlock-avoidance vs. retain's sorted cooccurrence upsert still holds.Measured (reproduced locally)
Seeded a bank the way the issue describes — hub entity degree 12,000, 14,000 cooccurrence rows (12,000 live hub edges + 2,000 genuinely-stale spoke pairs), on production table shapes/indexes:
INTERSECT)SubPlan HashSetOp Intersect, loops=14000, hub membership re-scanned per row (1.25M buffer hits)liveanti-join)livebuilt once, hash anti-join, no per-row subplanBoth delete the identical set (exactly the 2,000 stale pairs; all 12,000 hub pairs kept).
Bank-scoping matters on shared schemas — pruning a small bank next to large ones:
livescopeTests
test_prunes_cooccurrence_with_no_shared_unit,test_keeps_cooccurrence_with_shared_unit, and thetest_graph_maintenance_deadlock.pysuite (ordered-lock preserved).test_prunes_only_the_stale_edge_around_a_hub_and_leaves_other_banks— a hub's still-grounded edge survives while only its stale edge is pruned, and a second bank's stale edge is untouched (guards the bank-scoping).