Skip to content

Commit

Permalink
fix(backend): node_create_all duplicate AttributeValue rels
Browse files Browse the repository at this point in the history
If there are duplicate AttributeValue nodes present (due to concurrent
node creation for example), the MERGE statement would return more than
one node. Thus the HAS_VALUE relationship would be created for all the
nodes that have been returned.
Due to that behavior, attribute retrieval is slower because more
(duplicate) rows are returned in node_list_get_attribute.

Fix this by using a LIMIT 1 statement right after MERGE.

Using LIMIT 1 implies removing FOREACH statements which are not
compatible with LIMIT (that implies WITH). Move to CALL subqueries
instead to work that around.

CALL subqueries do not work very well with UNWIND when the list is
empty, make the whole query dynamic and remove parts when lists are
empty to work that around.

Signed-off-by: Fatih Acar <fatih@opsmill.com>
  • Loading branch information
fatih-acar committed Mar 5, 2025
1 parent eb60128 commit 2f2983d
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 19 deletions.
2 changes: 2 additions & 0 deletions backend/infrahub/core/query/attribute.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> No
query = """
MATCH (a:Attribute { uuid: $attr_uuid })
MERGE (av:%(labels)s { %(props)s } )
WITH av, a
LIMIT 1
CREATE (a)-[r:%(rel_label)s { branch: $branch, branch_level: $branch_level, status: "active", from: $at }]->(av)
""" % {"rel_label": self.attr._rel_to_value_label, "labels": ":".join(labels), "props": ", ".join(prop_list)}

Expand Down
85 changes: 66 additions & 19 deletions backend/infrahub/core/query/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,16 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
}
ipnetwork_prop_list = [f"{key}: {value}" for key, value in ipnetwork_prop.items()]

query = """
MATCH (root:Root)
CREATE (n:Node:%(labels)s $node_prop )
CREATE (n)-[r:IS_PART_OF $node_branch_prop ]->(root)
attrs_query = """
WITH distinct n
FOREACH ( attr IN $attrs |
UNWIND $attrs AS attr
CALL {
WITH n, attr
CREATE (a:Attribute { uuid: attr.uuid, name: attr.name, branch_support: attr.branch_support })
CREATE (n)-[:HAS_ATTRIBUTE { branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at }]->(a)
MERGE (av:AttributeValue { value: attr.content.value, is_default: attr.content.is_default })
WITH n, attr, av, a
LIMIT 1
CREATE (a)-[:HAS_VALUE { branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at }]->(av)
MERGE (ip:Boolean { value: attr.is_protected })
MERGE (iv:Boolean { value: attr.is_visible })
Expand All @@ -225,11 +226,19 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
MERGE (peer:Node { uuid: prop.peer_id })
CREATE (a)-[:HAS_OWNER { branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at }]->(peer)
)
)
FOREACH ( attr IN $attrs_iphost |
}"""

attrs_iphost_query = """
WITH distinct n
UNWIND $attrs_iphost AS attr_iphost
CALL {
WITH n, attr_iphost
WITH n, attr_iphost AS attr
CREATE (a:Attribute { uuid: attr.uuid, name: attr.name, branch_support: attr.branch_support })
CREATE (n)-[:HAS_ATTRIBUTE { branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at }]->(a)
MERGE (av:AttributeValue:AttributeIPHost { %(iphost_prop)s })
WITH n, attr, av, a
LIMIT 1
CREATE (a)-[:HAS_VALUE { branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at }]->(av)
MERGE (ip:Boolean { value: attr.is_protected })
MERGE (iv:Boolean { value: attr.is_visible })
Expand All @@ -243,11 +252,20 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
MERGE (peer:Node { uuid: prop.peer_id })
CREATE (a)-[:HAS_OWNER { branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at }]->(peer)
)
)
FOREACH ( attr IN $attrs_ipnetwork |
}
""" % {"iphost_prop": ", ".join(iphost_prop_list)}

attrs_ipnetwork_query = """
WITH distinct n
UNWIND $attrs_ipnetwork AS attr_ipnetwork
CALL {
WITH n, attr_ipnetwork
WITH n, attr_ipnetwork AS attr
CREATE (a:Attribute { uuid: attr.uuid, name: attr.name, branch_support: attr.branch_support })
CREATE (n)-[:HAS_ATTRIBUTE { branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at }]->(a)
MERGE (av:AttributeValue:AttributeIPNetwork { %(ipnetwork_prop)s })
WITH n, attr, av, a
LIMIT 1
CREATE (a)-[:HAS_VALUE { branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at }]->(av)
MERGE (ip:Boolean { value: attr.is_protected })
MERGE (iv:Boolean { value: attr.is_visible })
Expand All @@ -261,8 +279,14 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
MERGE (peer:Node { uuid: prop.peer_id })
CREATE (a)-[:HAS_OWNER { branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at }]->(peer)
)
)
FOREACH ( rel IN $rels_bidir |
}
""" % {"ipnetwork_prop": ", ".join(ipnetwork_prop_list)}

rels_bidir_query = """
WITH distinct n
UNWIND $rels_bidir AS rel
CALL {
WITH n, rel
MERGE (d:Node { uuid: rel.destination_id })
CREATE (rl:Relationship { uuid: rel.uuid, name: rel.name, branch_support: rel.branch_support })
CREATE (n)-[:IS_RELATED %(rel_prop)s ]->(rl)
Expand All @@ -279,8 +303,15 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
MERGE (peer:Node { uuid: prop.peer_id })
CREATE (rl)-[:HAS_OWNER { branch: rel.branch, branch_level: rel.branch_level, status: rel.status, from: $at }]->(peer)
)
)
FOREACH ( rel IN $rels_out |
}
""" % {"rel_prop": rel_prop_str}

rels_out_query = """
WITH distinct n
UNWIND $rels_out AS rel_out
CALL {
WITH n, rel_out
WITH n, rel_out as rel
MERGE (d:Node { uuid: rel.destination_id })
CREATE (rl:Relationship { uuid: rel.uuid, name: rel.name, branch_support: rel.branch_support })
CREATE (n)-[:IS_RELATED %(rel_prop)s ]->(rl)
Expand All @@ -297,8 +328,15 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
MERGE (peer:Node { uuid: prop.peer_id })
CREATE (rl)-[:HAS_OWNER { branch: rel.branch, branch_level: rel.branch_level, status: rel.status, from: $at }]->(peer)
)
)
FOREACH ( rel IN $rels_in |
}
""" % {"rel_prop": rel_prop_str}

rels_in_query = """
WITH distinct n
UNWIND $rels_in AS rel_in
CALL {
WITH n, rel_in
WITH n, rel_in AS rel
MERGE (d:Node { uuid: rel.destination_id })
CREATE (rl:Relationship { uuid: rel.uuid, name: rel.name, branch_support: rel.branch_support })
CREATE (n)<-[:IS_RELATED %(rel_prop)s ]-(rl)
Expand All @@ -315,14 +353,23 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
MERGE (peer:Node { uuid: prop.peer_id })
CREATE (rl)-[:HAS_OWNER { branch: rel.branch, branch_level: rel.branch_level, status: rel.status, from: $at }]->(peer)
)
)
}
""" % {"rel_prop": rel_prop_str}

query = f"""
MATCH (root:Root)
CREATE (n:Node:%(labels)s $node_prop )
CREATE (n)-[r:IS_PART_OF $node_branch_prop ]->(root)
{attrs_query if self.params["attrs"] else ""}
{attrs_iphost_query if self.params["attrs_iphost"] else ""}
{attrs_ipnetwork_query if self.params["attrs_ipnetwork"] else ""}
{rels_bidir_query if self.params["rels_bidir"] else ""}
{rels_out_query if self.params["rels_out"] else ""}
{rels_in_query if self.params["rels_in"] else ""}
WITH distinct n
MATCH (n)-[:HAS_ATTRIBUTE|IS_RELATED]-(rn)-[:HAS_VALUE|IS_RELATED]-(rv)
""" % {
"labels": ":".join(self.node.get_labels()),
"rel_prop": rel_prop_str,
"iphost_prop": ", ".join(iphost_prop_list),
"ipnetwork_prop": ", ".join(ipnetwork_prop_list),
}

self.params["at"] = at.to_string()
Expand Down
2 changes: 2 additions & 0 deletions backend/infrahub/core/query/resource_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: dict[str, Any]) -> No
query = """
MATCH (pool:%(number_pool)s { uuid: $pool_id })
MERGE (value:AttributeValue { value: $reserved, is_default: false })
WITH value, pool
LIMIT 1
CREATE (pool)-[rel:IS_RESERVED $rel_prop]->(value)
""" % {"number_pool": InfrahubKind.NUMBERPOOL}

Expand Down

0 comments on commit 2f2983d

Please sign in to comment.