fix(perf): kill Dashboard/timeline N+1 fan-out at fleet scale (#1265)#1269
Conversation
The Dashboard + timeline took 20s+ to paint metrics on a 10+ agent fleet.
Root cause was not the listed endpoints but blocking work serializing the
event loop and several per-agent/per-schedule N+1 query patterns:
- context-stats: `_fetch_single_agent_context` was `async` but ran a blocking
Docker container lookup + a blocking per-agent DB query on the event-loop
thread, so the `asyncio.gather` fan-out was effectively serial (10 agents x
~1-2s = the 20s). Now each coroutine does only the async HTTP call: the
container host is derived as `agent-{name}` (Docker DNS) with no Docker call,
and the latest activity is fetched once via a new bulk
`get_latest_activity_for_agents` window query.
- /api/agents/slots: replaced the per-agent Redis ZCARD loop with one pipelined
round-trip (mirrors heartbeat_status_bulk).
- /api/ops/schedules: replaced the per-schedule `get_schedule_executions` loop
with one `get_latest_execution_per_schedule` window query.
- /api/activities/timeline: pushed the per-user access filter into SQL
(agent_name IN ...) and dropped the limit*2 Python-side over-fetch; added a
standalone idx_activities_created index (schema.py + migration) so the
fleet-wide ORDER BY created_at DESC stops scan+sorting.
- Invariant #16: get_agent_health_history now uses iso_cutoff() instead of
datetime.utcnow(); same in context-stats activity cutoff.
No new cache layer, so no freshness/invalidation risk. New bulk methods are
covered by tests/unit/test_1265_dashboard_perf.py (SQLite + Postgres via the
#300 harness); window-function constructs verified on SQLite.
Related to #1265
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| ("agent_ownership_circuit_breaker", _migrate_agent_ownership_circuit_breaker), | ||
| ("voip_tables", _migrate_voip_tables), | ||
| ("operator_queue_cleared_at", _migrate_operator_queue_cleared_at), | ||
| ("activities_created_index", _migrate_activities_created_index), |
There was a problem hiding this comment.
[high] Missing the PostgreSQL track for this schema change. This adds idx_activities_created to db/schema.py + a SQLite migration, but there's no matching Alembic revision under src/backend/migrations/versions/ (only 0001_baseline.py exists). CLAUDE.md Rule #9 (#1183): "Every schema change requires TWO migrations" — SQLite and an Alembic revision.
Impact: an existing Postgres deployment is stamped at 0001_baseline and runs upgrade_to_head() on boot; with no 0002 revision, idx_activities_created is never created, so the fleet-wide ORDER BY created_at DESC timeline query keeps doing a scan+sort on Postgres — the exact #1265 regression this PR fixes for SQLite. Add an Alembic revision creating the index (and confirm 0001_baseline autogenerates it for fresh PG builds — it must be in tables.py MetaData, not just schema.py).
| order_by=schedule_executions.c.started_at.desc(), | ||
| ).label("rn") | ||
| subq = ( | ||
| select(schedule_executions, rn) |
There was a problem hiding this comment.
[med] select(schedule_executions, rn) pulls every column, including the large TEXT fields (response, tool_calls, execution_log — 100KB+ per row, message, error). The only caller (routers/ops.py last_execution) reads just id, status, started_at, completed_at, duration_ms, error.
This is the anti-pattern get_agent_executions_summary was written to avoid (PERF-001: project only needed columns). On a fleet with hundreds of schedules the single bulk query now streams tens of MB and deserializes full ScheduleExecution objects per /ops/schedules poll — partially eating the N+1 win it replaces. Project the 6 needed columns explicitly.
| # so we fetch exactly `limit` rows instead of over-fetching limit*2 and | ||
| # filtering in Python. | ||
| from services.agent_service.helpers import accessible_agent_names | ||
| allowed = accessible_agent_names(current_user) |
There was a problem hiding this comment.
[med] Admin timeline scope silently widened. Old code filtered the timeline against get_accessible_agents() (Docker-present agents only); the new admin path resolves to agent_names=None → no SQL filter. Activities of deleted agents (rows persist in agent_activities, container gone, so absent from list_all_agents_fast()) were previously excluded for admins and now appear.
Likely acceptable (admins seeing full audit history), but it changes what the admin dashboard shows. If intentional, worth a one-line note; if not, scope admins to known agents too.
| stats["contextPercent"] = session_data.get("context_percent", 0) | ||
| stats["contextUsed"] = session_data.get("context_tokens", 0) | ||
| stats["contextMax"] = session_data.get("context_window", 200000) | ||
| agent_url = f"http://agent-{agent_name}:8000/api/chat/session" |
There was a problem hiding this comment.
[low] Dropped the if container: existence check. The old code called get_agent_container(name) and skipped the HTTP call when it returned None. Now every agent whose cached status is "running" gets an unconditional client.get("http://agent-{name}:8000/...").
For a stale-running agent (container died/removed between the status snapshot and this call) each request now eats the full 2s httpx timeout before the except falls back to defaults, instead of skipping instantly — added latency + connection churn on exactly the unhealthy-fleet case the Dashboard matters most. gather bounds the total at ~2s, but it's a guaranteed 2s tail per dead host. (Also: the host string is now a hardcoded copy of the agent-{name}:8000 convention — consider a shared agent_base_url() helper.)
| with get_engine().connect() as conn: | ||
| return [self._row_to_activity(row) for row in conn.execute(stmt).all()] | ||
|
|
||
| def get_latest_activity_for_agents(self, agent_names: List[str]) -> Dict[str, Dict]: |
There was a problem hiding this comment.
[low/cleanup] Near-duplicate of ScheduleOperations.get_latest_execution_per_schedule. Both are "latest row per group" via ROW_NUMBER() OVER (PARTITION BY <key> ORDER BY <ts> DESC) WHERE rn=1 → fold into {key: row}, differing only in table/partition/order/mapper. Consider a shared _latest_per_group(...) helper so cross-backend window semantics (SQLite vs Postgres tie-breaking / NULL ordering) live in one place instead of drifting across two copies.
| ) | ||
| # Select only the activity columns (drop the rn helper) so positional | ||
| # row access in _row_to_activity stays aligned [0]..[14]. | ||
| stmt = select(*subq.c[: len(_ACTIVITY_COLUMNS)]).where(subq.c.rn == 1) |
There was a problem hiding this comment.
[low] Fragile positional projection. select(*subq.c[: len(_ACTIVITY_COLUMNS)]) exists only because _row_to_activity reads by integer position row[0]..row[14]. If anyone reorders/inserts/removes a column in _ACTIVITY_COLUMNS without updating the indices, the slice still "works" but maps wrong columns — silent data corruption, no exception. The schedules sibling avoids this by using .mappings() + name-based access; switching this method to the same would let it select by name and delete the slice.
Code review (high-effort, recall-biased) — 6 findingsReviewed the diff across 8 finder angles + verification. No crash-class bugs; the headline perf change is correct (event-loop de-blocking, bulk window queries, pipelined slots, SQL access filter,
#1 is the actionable blocker — the fix doesn't reach Postgres without an Alembic revision. Details inline. |
…ed helper Applies the six code-review findings on the #1265 dashboard perf PR: 1. [high] Add Alembic revision 0002_activities_created_index so EXISTING Postgres deployments (stamped at 0001_baseline, never re-running baseline) pick up idx_activities_created. Fresh PG builds already get it via 0001_baseline iterating db/schema.py:INDEXES. CLAUDE.md Rule #9 (dual-track). 2. [med] get_latest_execution_per_schedule now projects only the 6 dashboard fields instead of select(*) — no longer streams response/execution_log (100KB+)/tool_calls/message it never uses. Returns slim dicts (timestamps ISO-normalised to match the prior .isoformat() output); ops.py updated. 3. [med] Document that the admin timeline's unfiltered scope (full audit history incl. deleted agents) is intentional, not an accidental widening. 4. [low] Cap the context-stats HTTP connect timeout at 0.5s so a stale-`running` agent (container gone) fails fast instead of eating the full 2s read timeout — without reintroducing the per-agent Docker lookup that was the bottleneck. 5. [low] Extract the duplicated ROW_NUMBER window into a shared db/query_helpers.py:latest_per_group, reused by both bulk methods. 6. [low] get_latest_activity_for_agents maps rows by NAME via _mapping_to_activity (the helper drops the rn column), removing the fragile positional slice that would silently misalign if _ACTIVITY_COLUMNS were reordered. Tests: test_1265_dashboard_perf updated for the slim dict shape (+ asserts the narrow projection); 9 pass. 93 pass across affected suites. Alembic graph validated: single head 0002 -> 0001 -> base. Related to #1265 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review fixes applied —
|
|
Resolve by running |
vybe
left a comment
vybe
left a comment
There was a problem hiding this comment.
Validated via /validate-pr: P1 perf fix (#1265). Dual-track migration verified (schema.py + SQLite migrations.py + Alembic 0002 chained to 0001_baseline, all consistent); SQL parameterized, Inv #16 iso_cutoff fixed; 9 new tests + 125 pass on SQLite+PG. Remaining before/after-timing AC is non-blocking evidence. Approving.
2 participants
Summary
Fixes the 20s+ Dashboard/timeline metric load on 10+ agent fleets (#1265). Profiling showed the bottleneck was not the endpoints named in the issue but (a) blocking calls serializing the event loop on
context-stats, and (b) several per-agent / per-schedule N+1 query patterns that grow with fleet size.Root cause & fixes
context-stats(stats.py)_fetch_single_agent_contextwasasyncbut ran a blocking Docker container lookup + per-agent DB query on the event-loop thread →asyncio.gatherwas effectively serial (10 × ~1-2s = the 20s)agent-{name}(Docker DNS, no Docker call); latest activity fetched once via new bulkget_latest_activity_for_agentswindow query/api/agents/slotsZCARDloopheartbeat_status_bulk)/api/ops/schedulesget_schedule_executionsloopget_latest_execution_per_schedulewindow query/api/activities/timelineORDER BY created_at DESCwith nocreated_atindex;limit*2over-fetch + Python-side access filteridx_activities_created(schema.py + migration); access filter pushed into SQL (agent_name IN ...); exactlimitdb/monitoring.py,stats.pydatetime.utcnow()vs ISO-Z columns)iso_cutoff()No new cache layer → no freshness/invalidation risk (AC #5).
Testing
tests/unit/test_1265_dashboard_perf.py(9 tests) covering the three bulk methods + timeline access filter — runs on SQLite and Postgres via the Configurable database backend: SQLAlchemy Core abstraction (SQLite + PostgreSQL) #300 harness.Remaining AC
Related to #1265