Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 6 additions & 2 deletions backend/infrahub/prefect_server/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@


async def query_events(
session: AsyncSession, filter: EventFilter, page_size: int = INTERACTIVE_PAGE_SIZE, offset: int | None = None
session: AsyncSession,
filter: EventFilter,
page_size: int = INTERACTIVE_PAGE_SIZE,
offset: int | None = None,
include_total: bool = True,
) -> tuple[list[ReceivedEvent], int]:
count = await raw_count_events(session, filter) # type: ignore[attr-defined]
count = await raw_count_events(session, filter) if include_total else 0 # type: ignore[attr-defined]

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: Custom agent: Flag AI Slop and Fabricated Changes

When the caller omits count, this new branch skips the expensive aggregate and returns 0, but no regression test exercises that behavior. Add an event-query test without count that verifies the response count and prevents the count query from running.

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

<comment>When the caller omits `count`, this new branch skips the expensive aggregate and returns `0`, but no regression test exercises that behavior. Add an event-query test without `count` that verifies the response count and prevents the count query from running.</comment>

<file context>
@@ -12,9 +12,13 @@
+    include_total: bool = True,
 ) -> tuple[list[ReceivedEvent], int]:
-    count = await raw_count_events(session, filter)  # type: ignore[attr-defined]
+    count = await raw_count_events(session, filter) if include_total else 0  # type: ignore[attr-defined]
     page = await read_events(session, filter, limit=page_size, offset=offset)  # type: ignore[attr-defined]
     events = [ReceivedEvent.model_validate(e, from_attributes=True) for e in page]
</file context>

page = await read_events(session, filter, limit=page_size, offset=offset) # type: ignore[attr-defined]
events = [ReceivedEvent.model_validate(e, from_attributes=True) for e in page]
return events, count
14 changes: 13 additions & 1 deletion backend/infrahub/prefect_server/events.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from fastapi import APIRouter
from fastapi.param_functions import Depends
from prefect.server.database import PrefectDBInterface, provide_database_interface
from sqlalchemy import text

from .database import query_events
from .models import InfrahubEventfilterInput, InfrahubEventPage
Expand All @@ -18,8 +19,19 @@ async def read_events(
event_filter.filter.set_prefix()

async with db.session_context() as session:
if session.bind is not None and session.bind.dialect.name == "postgresql":
# After five executions of a prepared statement Postgres may switch it to a
# generic plan chosen without seeing the parameter values; for these event
# filters that plan degrades to a quadratic join that stalls for tens of
# seconds. Force per-execution planning for this transaction only, so the
# rest of the Prefect server keeps its prepared-statement plan caching.
await session.execute(text("SET LOCAL plan_cache_mode = force_custom_plan"))
events, total = await query_events(
session=session, filter=event_filter.filter, page_size=event_filter.limit, offset=event_filter.offset
session=session,
filter=event_filter.filter,
page_size=event_filter.limit,
offset=event_filter.offset,
include_total=event_filter.include_total,
)

return InfrahubEventPage(
Expand Down
4 changes: 4 additions & 0 deletions backend/infrahub/prefect_server/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,7 @@ class InfrahubEventfilterInput(BaseModel):
limit: int = Field(default=50)
filter: InfrahubEventFilter = Field(default_factory=InfrahubEventFilter.default)
offset: int | None = Field(default=None)
include_total: bool = Field(
default=True,
description="When false, skip the unbounded count query and report total=0; the paged events are unaffected",
)
25 changes: 22 additions & 3 deletions backend/infrahub/task_manager/event/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,17 +396,30 @@ async def query_events(
limit: int,
filters: InfrahubEventFilter,
offset: int | None = None,
include_total: bool = True,
) -> PrefectEventResponse:
body = {"limit": limit, "filter": filters.model_dump(mode="json", exclude_none=True), "offset": offset}
body = {
"limit": limit,
"filter": filters.model_dump(mode="json", exclude_none=True),
"offset": offset,
"include_total": include_total,
}

# Retry due to https://github.com/PrefectHQ/prefect/issues/16299
for _ in range(1, 5):
for attempt in range(1, 5):
prefect_error: PrefectHTTPStatusError | None = None
try:
response = await client._client.post("/infrahub/events/filter", json=body)
break
except PrefectHTTPStatusError as exc:
prefect_error = exc
# Each failed attempt can hide up to a full task-manager request timeout,
# so a silent loop here turns into a multi-minute stall for the caller.
log.warning(
"Event query to the task manager failed, retrying",

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.

P3: On the fourth failed POST, this warning says the query is retrying even though the loop has exhausted all attempts and raises after the sleep. Emit a terminal-failure message for attempt == 4 so logs distinguish retries from exhausted requests.

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

<comment>On the fourth failed POST, this warning says the query is retrying even though the loop has exhausted all attempts and raises after the sleep. Emit a terminal-failure message for `attempt == 4` so logs distinguish retries from exhausted requests.</comment>

<file context>
@@ -396,17 +396,30 @@ async def query_events(
+                # Each failed attempt can hide up to a full task-manager request timeout,
+                # so a silent loop here turns into a multi-minute stall for the caller.
+                log.warning(
+                    "Event query to the task manager failed, retrying",
+                    attempt=attempt,
+                    status_code=exc.response.status_code,
</file context>
Suggested change
"Event query to the task manager failed, retrying",
"Event query to the task manager failed, retrying"
if attempt < 4
else "Event query to the task manager failed",

attempt=attempt,
status_code=exc.response.status_code,
)
await asyncio.sleep(0.1)

if prefect_error:
Expand Down Expand Up @@ -438,8 +451,14 @@ async def query(
# returning data that will only be discarded
limit = 1

# The count is an unbounded aggregate over the whole filter window and is by far
# the most expensive part of the endpoint, so only ask for it when selected.
include_total = "count" in fields

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: Custom agent: Flag AI Slop and Fabricated Changes

The new selection-dependent include_total behavior has no regression coverage: existing event-query tests always request count, so they never verify that edge-only queries send include_total=False while count queries send True. Add focused assertions around the request payload or mocked query_events call for both selections.

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

<comment>The new selection-dependent `include_total` behavior has no regression coverage: existing event-query tests always request `count`, so they never verify that edge-only queries send `include_total=False` while count queries send `True`. Add focused assertions around the request payload or mocked `query_events` call for both selections.</comment>

<file context>
@@ -438,8 +451,14 @@ async def query(
 
+        # The count is an unbounded aggregate over the whole filter window and is by far
+        # the most expensive part of the endpoint, so only ask for it when selected.
+        include_total = "count" in fields
+
         async with get_client(sync_client=False) as client:
</file context>

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.

P3: include_total = "count" in fields misses an aliased selection (e.g. totalCount: count), because the field extractor keys the selection by field name, not alias. Such a query would silently receive count: 0 even though it requested the aggregate. Match on the underlying field definitions instead of the raw selection key.

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

<comment>`include_total = "count" in fields` misses an aliased selection (e.g. `totalCount: count`), because the field extractor keys the selection by field name, not alias. Such a query would silently receive `count: 0` even though it requested the aggregate. Match on the underlying field definitions instead of the raw selection key.</comment>

<file context>
@@ -438,8 +451,14 @@ async def query(
 
+        # The count is an unbounded aggregate over the whole filter window and is by far
+        # the most expensive part of the endpoint, so only ask for it when selected.
+        include_total = "count" in fields
+
         async with get_client(sync_client=False) as client:
</file context>


async with get_client(sync_client=False) as client:
response = await cls.query_events(client=client, filters=event_filter, limit=limit, offset=offset)
response = await cls.query_events(
client=client, filters=event_filter, limit=limit, offset=offset, include_total=include_total
)
nodes = [{"node": event.to_graphql()} for event in response.events]

return {"count": response.count, "edges": nodes}
1 change: 1 addition & 0 deletions changelog/+events-query-generic-plan-stall.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed intermittent multi-second to multi-minute stalls of the activity log. The task manager's Postgres connections switched the event queries to a generic prepared-statement plan after five executions, which degraded the unbounded count that backs every activity-log page from milliseconds to a quadratic join. The event-filter endpoint now forces per-execution planning for its own transaction only, and the count is only computed when the GraphQL query selects it.
25 changes: 25 additions & 0 deletions dev/knowledge/backend/events.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,31 @@ Events can be queried through:
- **REST API**: `/infrahub/events/filter` endpoint
- **Prefect Client**: Direct Prefect event API access

### Query-path performance constraints

The `/infrahub/events/filter` endpoint runs two SQL statements against the task manager's
Postgres: an unbounded `count(*)` over the whole filter window and the `LIMIT`-ed page read.
Two hard-earned constraints apply to this path:

- **The count is only computed when the caller asks for it.** The count aggregates every
matching row while the page read stops at the page size, so the count dominates the
endpoint's cost. The GraphQL resolver requests it (`include_total`) only when the query
selects `count` — the activity-log UI does not, so its page loads skip the aggregate
entirely. Keep that property when extending the endpoint.
- **The endpoint forces per-execution planning** (`SET LOCAL plan_cache_mode =
force_custom_plan` at the start of its transaction). After five executions of a
prepared statement, Postgres may switch it to a *generic* plan chosen without seeing
the parameter values. For these event filters — a wide `occurred` window plus a JSON
label match against `event_resources` — the generic plan degrades from a linear hash
join to a quadratic nested loop (measured: 5 ms → 3.5 s on a 5k-event table, growing
quadratically). The flip is per pool connection and per statement, which made the
resulting stalls look like a once-a-week CI flake: one pool connection runs the
pathological plan while its siblings answer in milliseconds. `SET LOCAL` scopes the
countermeasure to this transaction only — the rest of the Prefect server keeps its
prepared-statement plan caching — at the cost of replanning these two queries per
request (~1.5 ms). Do not remove it without re-checking the event queries' plans under
`plan_cache_mode = force_generic_plan`.

## Key Locations

| Component | Location |
Expand Down
Loading