Skip to content
Open
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
14 changes: 14 additions & 0 deletions backend/infrahub/core/diff/model/path.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,20 @@ def add_relationship_node_uuid(self, name: str, node_uuid: str) -> None:
"""Record that `node_uuid` is a node of this kind whose relationship `name` changed."""
self.relationship_node_uuids.setdefault(name, set()).add(node_uuid)

def merge(self, other: NodeDiffFieldSummary) -> None:
"""Fold another summary of the same kind into this one.

Raises:
ValueError: If the other summary is for a different kind.

"""
if other.kind != self.kind:
raise ValueError(f"Cannot merge summary for kind {other.kind} into summary for kind {self.kind}")
for name, node_uuids in other.attribute_node_uuids.items():
self.attribute_node_uuids.setdefault(name, set()).update(node_uuids)
for name, node_uuids in other.relationship_node_uuids.items():
self.relationship_node_uuids.setdefault(name, set()).update(node_uuids)

@property
def attribute_names(self) -> set[str]:
return set(self.attribute_node_uuids)
Expand Down
93 changes: 44 additions & 49 deletions backend/infrahub/core/diff/query/field_summary.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from dataclasses import dataclass
from collections.abc import Generator
from typing import Any

from infrahub.core.constants import DiffAction
Expand All @@ -8,29 +8,30 @@
from ..model.path import NodeDiffFieldSummary, TrackingId


@dataclass
class FieldNodeUuidsRow:
"""One changed field of a kind and the uuids of the nodes that changed it, as projected by the query."""

name: str
node_uuids: list[str]


class EnrichedDiffNodeFieldSummaryQuery(Query):
"""Get node kind and names of all altered attributes and relationships for each kind."""
"""Get the names of all altered attributes and relationships for one page of changed nodes.

Pagination is over the changed nodes, strictly ordered, with each row carrying every altered
field name of one node; aggregating the fields of every node in one transaction does not scale
with large diffs.
"""

name = "enriched_diff_node_field_summary"
type = QueryType.READ

def __init__(
self,
diff_branch_name: str,
node_offset: int,
node_limit: int,
tracking_id: TrackingId | None = None,
diff_id: str | None = None,
**kwargs: Any,
) -> None:
super().__init__(**kwargs)
self.diff_branch_name = diff_branch_name
self.node_offset = node_offset
self.node_limit = node_limit
self.tracking_id = tracking_id
self.diff_id = diff_id

Expand All @@ -42,6 +43,8 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa
"diff_branch": self.diff_branch_name,
"tracking_id": self.tracking_id.serialize() if self.tracking_id else None,
"diff_id": self.diff_id,
"node_offset": self.node_offset,
"node_limit": self.node_limit,
}
query = """
MATCH (diff_root:DiffRoot)
Expand All @@ -51,48 +54,40 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # noqa
AND (diff_root.uuid = $diff_id OR $diff_id IS NULL)
OPTIONAL MATCH (diff_root)-[:DIFF_HAS_NODE]->(n:DiffNode)
WHERE n.action <> $unchanged_str
WITH DISTINCT diff_root, n.kind AS kind
CALL (diff_root, kind) {
OPTIONAL MATCH (diff_root)-[:DIFF_HAS_NODE]->(n:DiffNode {kind: kind})-[:DIFF_HAS_ATTRIBUTE]->(a:DiffAttribute)
WHERE n.action <> $unchanged_str
AND a.action <> $unchanged_str
WITH a.name AS attr_name, collect(DISTINCT n.uuid) AS attr_node_uuids
WHERE attr_name IS NOT NULL
RETURN collect({name: attr_name, node_uuids: attr_node_uuids}) AS attr_name_uuids
WITH n
ORDER BY n.uuid, elementId(n)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Large diffs can still consume unbounded query work/memory as offsets grow: the final page must order nearly every changed node before LIMIT applies. Use keyset pagination (last (uuid, elementId) cursor) so each query seeks after the previous node and limits immediately.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/core/diff/query/field_summary.py, line 58:

<comment>Large diffs can still consume unbounded query work/memory as offsets grow: the final page must order nearly every changed node before `LIMIT` applies. Use keyset pagination (last `(uuid, elementId)` cursor) so each query seeks after the previous node and limits immediately.</comment>

<file context>
@@ -51,48 +54,40 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None:  # noqa
-            WHERE attr_name IS NOT NULL
-            RETURN collect({name: attr_name, node_uuids: attr_node_uuids}) AS attr_name_uuids
+        WITH n
+        ORDER BY n.uuid, elementId(n)
+        SKIP $node_offset
+        LIMIT $node_limit
</file context>

SKIP $node_offset
LIMIT $node_limit
CALL (n) {
OPTIONAL MATCH (n)-[:DIFF_HAS_ATTRIBUTE]->(a:DiffAttribute)
WHERE a.action <> $unchanged_str
RETURN collect(DISTINCT a.name) AS attr_names
}
WITH diff_root, kind, attr_name_uuids
CALL (diff_root, kind) {
OPTIONAL MATCH (diff_root)-[:DIFF_HAS_NODE]->(n:DiffNode {kind: kind})-[:DIFF_HAS_RELATIONSHIP]->(r:DiffRelationship)
WHERE n.action <> $unchanged_str
AND r.action <> $unchanged_str
WITH r.name AS rel_name, collect(DISTINCT n.uuid) AS rel_node_uuids
WHERE rel_name IS NOT NULL
RETURN collect({name: rel_name, node_uuids: rel_node_uuids}) AS rel_name_uuids
CALL (n) {
OPTIONAL MATCH (n)-[:DIFF_HAS_RELATIONSHIP]->(r:DiffRelationship)
WHERE r.action <> $unchanged_str
RETURN collect(DISTINCT r.name) AS rel_names
}
"""
self.add_to_query(query=query)
self.order_by = ["kind"]
self.return_labels = ["kind", "attr_name_uuids", "rel_name_uuids"]
self.return_labels = ["n.kind AS kind", "n.uuid AS node_uuid", "attr_names", "rel_names"]

async def get_field_summaries(self) -> list[NodeDiffFieldSummary]:
field_summaries = []
for result in self.get_results():
kind = result.get_as_type(label="kind", return_type=str)
attribute_node_uuids = self._to_field_uuids(
result.get_as_list_of_type(label="attr_name_uuids", return_type=FieldNodeUuidsRow)
)
relationship_node_uuids = self._to_field_uuids(
result.get_as_list_of_type(label="rel_name_uuids", return_type=FieldNodeUuidsRow)
)
if attribute_node_uuids or relationship_node_uuids:
field_summaries.append(
NodeDiffFieldSummary(
kind=kind,
attribute_node_uuids=attribute_node_uuids,
relationship_node_uuids=relationship_node_uuids,
)
)
return field_summaries
def get_node_field_rows(self) -> Generator[NodeDiffFieldSummary, None, None]:
"""Yield a single-node field summary for each changed node in this page.

def _to_field_uuids(self, rows: list[FieldNodeUuidsRow]) -> dict[str, set[str]]:
return {row.name: set(row.node_uuids) for row in rows}
A node whose fields are all unchanged still yields a summary (with empty field maps) so the
caller can count the nodes consumed from this page. A diff root with no changed nodes at all
produces one node-less row instead; it is skipped here, which is safe for the caller's
consumed-node count because null nodes sort after every real node.
"""
for result in self.get_results():
kind = result.get_as_str("kind")
node_uuid = result.get_as_str("node_uuid")
if not kind or not node_uuid:
continue
summary = NodeDiffFieldSummary(kind=kind)
for attr_name in result.get_as_type(label="attr_names", return_type=list[str]):
summary.add_attribute_node_uuid(name=attr_name, node_uuid=node_uuid)
for rel_name in result.get_as_type(label="rel_names", return_type=list[str]):
summary.add_relationship_node_uuid(name=rel_name, node_uuid=node_uuid)
yield summary
31 changes: 26 additions & 5 deletions backend/infrahub/core/diff/repository/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -588,11 +588,32 @@ async def get_affected_node_uuids(
async def get_node_field_summaries(
self, diff_branch_name: str, tracking_id: TrackingId | None = None, diff_id: str | None = None
) -> list[NodeDiffFieldSummary]:
query = await EnrichedDiffNodeFieldSummaryQuery.init(
db=self.db, diff_branch_name=diff_branch_name, tracking_id=tracking_id, diff_id=diff_id
)
await query.execute(db=self.db)
return await query.get_field_summaries()
node_limit = config.SETTINGS.database.query_size_limit
node_offset = 0
summaries_by_kind: dict[str, NodeDiffFieldSummary] = {}
while True:
query = await EnrichedDiffNodeFieldSummaryQuery.init(
db=self.db,
diff_branch_name=diff_branch_name,
tracking_id=tracking_id,
diff_id=diff_id,
node_offset=node_offset,
node_limit=node_limit,
)
await query.execute(db=self.db)
num_nodes = 0
for node_summary in query.get_node_field_rows():
num_nodes += 1
if not node_summary.attribute_node_uuids and not node_summary.relationship_node_uuids:
continue
kind_summary = summaries_by_kind.setdefault(
node_summary.kind, NodeDiffFieldSummary(kind=node_summary.kind)
)
kind_summary.merge(node_summary)
if num_nodes < node_limit:
break
node_offset += node_limit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Very large diffs can still cause progressively larger query work and heap use: offset pagination makes the last ORDER BY ... SKIP page process nearly every changed node. Cursor/keyset pagination using the last (uuid, elementId) would keep each query bounded and meet the stated memory goal.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/infrahub/core/diff/repository/repository.py, line 615:

<comment>Very large diffs can still cause progressively larger query work and heap use: offset pagination makes the last `ORDER BY ... SKIP` page process nearly every changed node. Cursor/keyset pagination using the last `(uuid, elementId)` would keep each query bounded and meet the stated memory goal.</comment>

<file context>
@@ -588,11 +588,32 @@ async def get_affected_node_uuids(
+                kind_summary.merge(node_summary)
+            if num_nodes < node_limit:
+                break
+            node_offset += node_limit
+        return list(summaries_by_kind.values())
 
</file context>

return list(summaries_by_kind.values())

async def mark_tracking_ids_merged(self, tracking_ids: list[TrackingId]) -> None:
query = await EnrichedDiffMergedTrackingIdQuery.init(db=self.db, tracking_ids=tracking_ids)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from dataclasses import dataclass
from typing import Generator

import pytest
Expand Down Expand Up @@ -25,6 +26,19 @@
from .base import DiffRepositoryTestBase


@dataclass
class PageSizeCase:
name: str
query_size_limit: int


PAGE_SIZE_CASES = [
PageSizeCase(name="partial_last_page", query_size_limit=4),
PageSizeCase(name="node_count_exact_multiple_of_page", query_size_limit=5),
PageSizeCase(name="single_page", query_size_limit=50),
]


class TestDiffNodeFieldSummaries(DiffRepositoryTestBase):
base_branch_name: str = "main"
diff_branch_name: str = "diff"
Expand Down Expand Up @@ -176,3 +190,84 @@ async def test_get_node_field_summaries_excludes_other_diffs(
diff_branch_name=requested_branch_name, diff_id=requested_diff.uuid
)
assert retrieved_by_diff_id == expected_summaries

async def test_get_node_field_summaries_empty_diff(
self, diff_repository: DiffRepository, reset_database: None
) -> None:
"""A diff root with no diff nodes at all yields no summaries."""
tracking_id = BranchTrackingId(name=self.diff_branch_name)
enriched_diff = EnrichedRootFactory.build(
base_branch_name=self.base_branch_name,
diff_branch_name=self.diff_branch_name,
from_time=self.diff_from_time,
to_time=self.diff_to_time,
nodes=set(),
tracking_id=tracking_id,
)
await self._save_single_diff(
diff_repository=diff_repository, enriched_diff=enriched_diff, do_summary_counts=False
)

retrieved = await diff_repository.get_node_field_summaries(
diff_branch_name=self.diff_branch_name, tracking_id=tracking_id
)
assert retrieved == []

@pytest.mark.parametrize("case", PAGE_SIZE_CASES, ids=lambda c: c.name)
async def test_get_node_field_summaries_batched(
self, diff_repository: DiffRepository, reset_database: None, case: PageSizeCase
) -> None:
"""Per-kind summaries are complete when the changed nodes span multiple retrieval pages.

Ten changed nodes of two kinds are saved, so each page size below ten forces every kind
across a page boundary; one node has only unchanged fields, so it fills a page slot without
contributing a summary.
"""
tracking_id = BranchTrackingId(name=self.diff_branch_name)
kinds = ["TestingKindAlpha", "TestingKindBravo"]
nodes: set[EnrichedDiffNode] = set()
expected_by_kind: dict[str, NodeDiffFieldSummary] = {}
for index in range(9):
kind = kinds[index % len(kinds)]
node = self._build_named_field_node(
kind=kind,
node_action=DiffAction.UPDATED,
attribute_actions={
"shared_attr": DiffAction.UPDATED,
f"attr_{index}": DiffAction.ADDED,
"quiet_attr": DiffAction.UNCHANGED,
},
relationship_actions={"shared_rel": DiffAction.UPDATED},
)
nodes.add(node)
expected = expected_by_kind.setdefault(kind, NodeDiffFieldSummary(kind=kind))
expected.add_attribute_node_uuid(name="shared_attr", node_uuid=node.uuid)
expected.add_attribute_node_uuid(name=f"attr_{index}", node_uuid=node.uuid)
expected.add_relationship_node_uuid(name="shared_rel", node_uuid=node.uuid)
nodes.add(
self._build_named_field_node(
kind=kinds[0],
node_action=DiffAction.UPDATED,
attribute_actions={"quiet_attr": DiffAction.UNCHANGED},
relationship_actions={"quiet_rel": DiffAction.UNCHANGED},
)
)
enriched_diff = EnrichedRootFactory.build(
base_branch_name=self.base_branch_name,
diff_branch_name=self.diff_branch_name,
from_time=self.diff_from_time,
to_time=self.diff_to_time,
nodes=nodes,
tracking_id=tracking_id,
)
await self._save_single_diff(
diff_repository=diff_repository, enriched_diff=enriched_diff, do_summary_counts=False
)

config.SETTINGS.database.query_size_limit = case.query_size_limit
retrieved = await diff_repository.get_node_field_summaries(
diff_branch_name=self.diff_branch_name, tracking_id=tracking_id
)

assert len(retrieved) == len(expected_by_kind)
assert {summary.kind: summary for summary in retrieved} == expected_by_kind
Loading