Repository navigation
fix(ontology): prevent duplicate Ontology nodes from concurrent ingestion - #596
Merged
Merged
Conversation
AGE has no uniqueness constraints, so the check-then-CREATE in ensure_ontology_exists let two concurrent ingestion workers both see a new ontology as missing and both create it. That left two Ontology nodes named operating-models, which ontology list and scores then show twice. Run the re-check and the CREATE on one connection holding a per-name session advisory lock, committing before unlock so the next waiter sees the node. _execute_cypher gains an optional caller-held conn for this. Refs #588
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
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.
Refs #588 (the existing duplicate nodes still need repair; follow-ups in #597)
Root cause
It's a data duplicate, not a query fan-out. Read-only Cypher on the dev graph shows two
:Ontologynodes namedoperating-models:The two ids are consecutive, both nodes were created at epoch 6, and each has 11 incoming
SCOPED_BYedges from Sources. Edge creation matches the ontology by name, so it links each Source to both nodes. The list and scores endpoints just return every node.The cause is
ensure_ontology_exists: it checks and then CREATEs, and AGE has no uniqueness constraint. Two ingestion workers starting on a new ontology both see it as missing and both create it. The existing "race" handler only runs if CREATE raises, and CREATE never raises here. The nodes date from epoch 6, so the epoch-22 CLEAVE suggested in the issue comment isn't the cause.Fix
ensure_ontology_existstakes a per-name session advisory lock (pg_advisory_lock(200, hashtext(name))) on one connection. Under that lock it re-checks, CREATEs, and commits before it unlocks. If the unlock fails, the connection is discarded rather than returned to the pool._execute_cypher(andget_ontology_node/create_ontology_node) accept an optional caller-heldconn.Not in this PR: repairing existing data
The duplicate node is still in the graph. A repair would keep one node (the lower id), move the
SCOPED_BYedges and anything keyed by the otherontology_id(scores, annealing proposals) onto it, thenDETACH DELETEthe loser. That is destructive, so I've left it for a separate decision. A later uniqueness guard (an expression index onOntologyname) can only go in after the repair.Test plan
TestEnsureOntologyExistsConcurrency: two threads ensure the same new ontology. On pre-fix code it fails (2 nodes created); with the fix it passes. It also checks that the lock is released when CREATE fails.pytest tests/unit/: 881 passed (run inkg-api-devon a copy of this branch)pytest tests/api/ -k "ontolog or batch": 103 passed, 5 skippedlint_queries.py: clean