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
143 changes: 143 additions & 0 deletions changelogs/v0.7.0/briancastelino_2026-07-09.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
## Stop broad Graph Chat queries from freezing the whole app (issue #114)

### Context
A single Graph Chat question that triggered a broad graph read (deep BFS
traversal, unfiltered `triples/find`, or a heavy GraphQL resolver) could make
the entire OntoBricks app unresponsive for every user until the query finished
or the browser gave up. Two root causes compounded each other:

1. **Event-loop starvation.** The internal `/dtwin/...` graph routes the chat
agent calls over loopback ran their blocking SQL *directly on the single
uvicorn event loop*. While one slow query ran, no other request — health
checks, other users' pages, other chat turns — could be serviced.
2. **Unbounded reads.** Nothing capped how long a graph read could run or how
many triples it could return, so a runaway query pinned a DB session (and,
through the loopback agent, the event loop) indefinitely.

The fix bounds every graph read server-side, offloads the blocking work off the
event loop, auto-sizes the worker pool to the instance, and — when the app is
genuinely saturated — tells the user why and what to do about it, with an admin
knob to tune the bounds.

### Changes
1. `src/back/core/query_limits.py` (new)
Central resolver for the two graph-read bounds — statement timeout and Graph
Chat result cap — with admin-override → env-var → built-in-default precedence
and clamping so a misconfiguration can't disable the guard.
2. `src/back/core/graphdb/lakebase/LakebaseFlatStore.py`
`execute_query` sets a per-read `SET statement_timeout` (ms) from
`get_graph_query_timeout_s()` so a runaway read is cancelled server-side
instead of pinning the pooled connection.
3. `src/back/core/graphdb/lakebase/LakebaseBase.py`
The DDL/migration `_cursor` resets `SET statement_timeout = 0` so schema work
on a reused pooled connection isn't cancelled by a prior read's bound.
4. `src/back/core/databricks/SQLWarehouse.py`
`execute_query` gained an optional `statement_timeout_s`: when positive it
brackets the query with `SET STATEMENT_TIMEOUT` and resets it to `0` on the
pooled session afterwards.
5. `src/back/core/triplestore/delta/DeltaTripleStore.py`
Routes bounded graph reads through `SQLWarehouse.execute_query` with the
resolved timeout; full-graph dumps stay unbounded.
6. `src/api/routers/internal/dtwin.py`
Offloaded the blocking DB work in `triples/find`, `graphql/execute`,
`neighbors`, and `sync/stats` into `run_blocking` (extract-function of the
blocking body); `triples/find` now clamps `limit` to the result cap. Added
`_resource_pressure_payload()` and merged it into both chat responses
(`/assistant/chat` and the SSE `done` event).
7. `src/back/core/helpers/DatabricksHelpers.py` (+ `__init__.py`)
Auto-sizes the blocking `ThreadPoolExecutor` from vCPU count (`cpu*4`, floored
at the historical 20, still overridable via `ONTOBRICKS_THREAD_POOL_SIZE`),
tracks in-flight tasks in `run_blocking`, and exposes `get_blocking_pool_stats()`
with a `saturated` flag.
8. `src/agents/agent_dtwin_chat/tools.py`
The loopback HTTP client timeout is now derived from the server statement
timeout plus a margin, so a bounded query is cancelled server-side (clean
error the LLM can react to) before the client aborts.
9. `src/back/objects/session/GlobalConfigService.py`
Persists/applies `graph_query_timeout_s` and `graph_chat_result_cap`
(get/set + `_empty()` keys + re-apply on config load; `0` = unset).
10. `src/back/objects/domain/SettingsService.py`
`get_graph_limits_result` / `save_graph_limits_result` (admin-gated).
11. `src/api/routers/internal/settings.py`
`GET /settings/graph-limits` and `POST /settings/save-graph-limits`.
12. `src/front/templates/settings.html`, `src/front/static/config/js/settings.js`
Admin-only "Graph read limits" inputs on the Graph DB tab (load + save).
13. `src/front/static/query/js/query-chat.js`
Inline advisory bubble + global toast when the server flags resource pressure.
14. `docs/issues/graph-chat-event-loop-hang.md` (new)
Bug write-up: symptom, root cause, and remediation.

### Files modified
- `src/back/core/query_limits.py`
- `src/back/core/graphdb/lakebase/LakebaseFlatStore.py`
- `src/back/core/graphdb/lakebase/LakebaseBase.py`
- `src/back/core/databricks/SQLWarehouse.py`
- `src/back/core/triplestore/delta/DeltaTripleStore.py`
- `src/api/routers/internal/dtwin.py`
- `src/api/routers/internal/settings.py`
- `src/back/core/helpers/DatabricksHelpers.py`
- `src/back/core/helpers/__init__.py`
- `src/agents/agent_dtwin_chat/tools.py`
- `src/back/objects/session/GlobalConfigService.py`
- `src/back/objects/domain/SettingsService.py`
- `src/front/templates/settings.html`
- `src/front/static/config/js/settings.js`
- `src/front/static/query/js/query-chat.js`
- `docs/issues/graph-chat-event-loop-hang.md`
- `tests/units/core/test_query_limits.py` (new)
- `tests/units/core/test_graph_query_bounds.py` (new)

### Related work
Contributor @ulsmo (issue #114 comment) confirmed the bug and traced the
triggers on a 1–5M-triple graph to two performance bottlenecks this change
complements but does not replace: #112 (indexes dropped/not applied during
Lakeflow sync → unindexed `_sync` tables → exploding query times) and PR #115
(optimizes the `expand_uri_aliases` / `get_triples_for_subjects` path that
`triples/find` invokes). This change is the resilience layer: it bounds and
offloads reads so a slow query degrades to a clean per-request cancel instead of
an app-wide freeze. Trade-off: on an unindexed large graph a legitimate read may
now hit the default 60s timeout and be cancelled; the admin knob (≤900s) covers
that until #112/#115 land.

### Review fixups (Copilot, PR #116)
- **[High] Lakebase statement_timeout leak.** The pool hands out
`autocommit=True` connections, so the per-read `SET statement_timeout` in
`LakebaseFlatStore.execute_query` persisted to the next borrower and could
cancel a bulk COPY/INSERT or DDL mid-write. Now wrapped in `try/finally` with
`RESET statement_timeout` so the bound is scoped to the read (safe in
autocommit — a timeout cancel leaves no open transaction to abort).
- **[Med] Settings input validation.** `save-graph-limits` `_opt_int` now raises
`ValidationError` on non-integer input instead of letting `int()` bubble as a
500.
- **[Med] Config/UI vs runtime consistency.** `GlobalConfigService`
get/set for `graph_query_timeout_s` and `graph_chat_result_cap` now route the
persisted value through the central clamp (`back.core.query_limits`) so the
Settings UI never shows or re-saves an out-of-range value that differs from the
enforced effective bound.
- Added `TestLakebaseStatementTimeoutReset` (reset on success and on error) to
`tests/units/core/test_graph_query_bounds.py`.

### Rebase onto `develop` (v0.7.0, PR #116)
Retargeted from `master` to the `develop` integration branch. `develop` had
already migrated `triplestore` → `graphdb` and added its own `run_blocking`
offloading for the graph routes, so the redundant offloading half of this change
was dropped in favour of develop's, and only the unique bounding/advisory layer
was kept:
- `triplestore/delta/DeltaTripleStore.py` was deleted on `develop`; the bounded
read was ported to `graphdb/delta/DeltaFlatStore.execute_query`.
- `dtwin.py` kept only the resource-pressure advisory (`_resource_pressure_payload`
in the chat + SSE responses) and the `get_graph_chat_result_cap()` clamp on
`triples/find`; the `run_blocking` wrapping is now inherited from `develop`.
- `settings.js` kept only `loadGraphLimits()` + the save handler; the Graph DB tab
refactor is `develop`'s.
- Changelog moved from `v0.6.1/` to `v0.7.0/` to match the `develop` version line.
- `TestDeltaStoreBoundsReads` updated to target `DeltaFlatStore`.

### Tests
`uv` is not installed in this dev environment and system Python lacks `psycopg`,
so the full `uv run pytest -q -m "not scenario"` suite could not be run here.
Ran the targeted subset with system Python + pytest:
`python -m pytest tests/units/core/test_query_limits.py tests/units/core/test_graph_query_bounds.py tests/units/core/test_sql_warehouse.py -q`
→ **41 passed** (includes the pre-existing `test_sql_warehouse.py` and the two new
Lakebase timeout-reset tests — no regression). Full-suite run still required in a
`uv`/`psycopg` environment before merge.
114 changes: 114 additions & 0 deletions docs/issues/graph-chat-event-loop-hang.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# [BUG]: Graph Chat heavy query blocks the event loop and freezes the app until redeploy

> Tracked as GitHub issue #114. Formatted to match `.github/ISSUE_TEMPLATE/bug.yml`.

## Is there an existing issue for this?

- [x] I have searched the existing issues

## Current Behavior

Asking Graph Chat a broad question (e.g. *"What are the top 10 violations?"*) hangs
the request and then freezes the **entire** app: no other request — from any user —
completes until the app is redeployed.

Root cause: the Graph Chat agent runs in a worker thread and makes **synchronous
loopback HTTP calls** back into the same FastAPI process. The internal `/dtwin/...`
routes it hits (`dtwin_triples_find`, `dtwin_graphql_execute`, `dtwin_neighbors`,
`sync/stats`) are declared `async def` but execute **blocking DB work directly on the
asyncio event loop** — notably the recursive BFS CTE in
`TripleStoreBackend.bfs_traversal`. That query joins on
`(t.subject = b.entity OR t.object = b.entity)` with no `LIMIT` and no DB
`statement_timeout`, so a broad seed explodes into a multi-minute / effectively
unbounded query. Because it runs on the event loop, the single uvicorn worker is
frozen for the whole query, stalling every other request. The agent's httpx client
timeout fires client-side (`triples/find error: timed out`) but does **not** cancel
the server-side query, so the loop stays blocked and the Lakebase connection pool
(`_POOL_MAX_SIZE = 4`) gets pinned.

This violates the project's own guidance in `src/.coding_rules.md`
("synchronous I/O in an async handler blocks the event loop … wrap it in
`asyncio.to_thread`"). The helper already exists: `DatabricksHelpers.run_blocking()`.

## Expected Behavior

- Slow/broad questions time out **gracefully** with an error message the agent can
react to, cancelled server-side by a `statement_timeout`.
- The app stays responsive to all other users/requests while a heavy Graph Chat
query runs (blocking DB work is offloaded off the event loop).
- Under sustained load, the app advises upgrading the Databricks App instance size
instead of silently freezing.

## Steps To Reproduce

1. Create / load a domain and build its Knowledge Graph (Lakebase backend).
2. Open the **Graph Chat** tab.
3. Ask a broad question, e.g. *"What are the top 10 violations?"*.
4. Observe the request hang; then observe that all other pages/requests also stall
until the app is redeployed.

## Cloud

<!-- AWS / Azure / GCP -->

## Browser

<!-- Chrome / Firefox / Edge / Safari / Other -->

## OntoBricks Version

0.6.1

## Relevant log output

```shell
WARNING agents.agent_graph_chat_tools | tools._error:71 | agent_dbx_chat: triples/find error: timed out
INFO uvicorn.access | 'POST /dtwin/graphql/execute HTTP/1.1' 200
# ... after the timeout, no further requests are served until redeploy
```

## Additional Context

- Graph backend: Lakebase Postgres (the same class of bug affects the Delta/SQL
warehouse backend, which likewise has no per-statement cancellation — only a
30s socket timeout).
- Deployed logs reference `agents.agent_graph_chat_tools` / `agent_dbx_chat`, which
are renamed to `agent_dtwin_chat` on current `main`; the architectural bug is
identical on `main`.

## Related issues (query-performance layer)

This issue is the **resilience / graceful-degradation** half of the problem. The
contributor **@ulsmo** confirmed the bug and traced the *triggers* on a
1–5M-triple graph to two underlying performance bottlenecks — this fix bounds
their blast radius but does not replace them:

- **#112** — Indexes dropped/not applied during Lakeflow sync, leaving `_sync`
tables unindexed and causing exploding query times. A root cause of the slow
reads that trip this hang.
- **#115** — `Perf(graphdb): Optimize finding subjects with local id's` (PR):
optimizes `describe_entity` alias expansion on large ID sets — the same
`expand_uri_aliases` / `get_triples_for_subjects` path the `triples/find` route
invokes.

These are complementary: even with #112 and #115 fixed, an unbounded read can
still starve the single event loop, so the bounding + offloading here is still
required.

## Proposed Fix

1. Offload the blocking DB work in the internal `/dtwin` graph routes via
`run_blocking(...)` so it never runs on the event loop.
2. Add a configurable graph-read `statement_timeout` on **both** backends
(Lakebase `SET statement_timeout`, warehouse `SET STATEMENT_TIMEOUT`) so a runaway
query is cancelled server-side.
3. Auto-tune the blocking thread pool from the instance size and surface an
admin-configurable advisory recommending an instance-size upgrade under pressure.

### Caveat / trade-off

On an **unindexed** large graph (see #112), a legitimate `describe` / `triples/find`
can now exceed the default 60s `statement_timeout` and be **cancelled** — turning a
*silent app-wide freeze* into a *clean, per-request "query cancelled" error*. This
is the intended trade-off; the admin timeout knob (up to 900s) lets operators raise
the bound until #112/#115 fix the underlying query speed.
21 changes: 18 additions & 3 deletions src/agents/agent_dtwin_chat/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@

logger = get_logger(__name__)

_HTTP_TIMEOUT = 120
_HTTP_TIMEOUT = 120 # fallback floor when the graph timeout can't be resolved
_HTTP_TIMEOUT_MARGIN_S = 30
_MAX_DEPTH = 1
_SPARQL_DANGEROUS = re.compile(
r"\b(DROP|DELETE|INSERT|CREATE|CLEAR|LOAD|COPY|MOVE|ADD)\b",
Expand All @@ -61,8 +62,22 @@


def _client(ctx: ToolContext):
"""Build a sync HTTP client bound to the loopback OntoBricks URL."""
return loopback_client(ctx, timeout=_HTTP_TIMEOUT)
"""Build a sync HTTP client bound to the loopback OntoBricks URL.

The timeout is derived from the server-side graph statement timeout plus a
margin, so a bounded query is cancelled *server-side* (returning a clean
error the LLM can react to) before the loopback client gives up on a query
that is still running.
"""
try:
from back.core.query_limits import get_graph_query_timeout_s

timeout = max(
_HTTP_TIMEOUT, get_graph_query_timeout_s() + _HTTP_TIMEOUT_MARGIN_S
)
except Exception: # noqa: BLE001
timeout = _HTTP_TIMEOUT
return loopback_client(ctx, timeout=timeout)


def _registry_params(ctx: ToolContext) -> dict:
Expand Down
36 changes: 35 additions & 1 deletion src/api/routers/internal/dtwin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1832,6 +1832,37 @@ async def get_inferred_triples(
_CHAT_MIN_LIMIT = 5
_CHAT_MAX_LIMIT = 100

# Surfaced to the Graph Chat UI (inline + global toast) when the blocking
# thread pool is saturated, so users understand why responses are slow and
# admins know the actionable remedy.
_UPGRADE_INSTANCE_ADVICE = (
"OntoBricks is under heavy load - the request worker pool is saturated, so "
"responses may be slow. If this happens often, upgrade the Databricks App "
"instance size (Apps UI -> Compute) for more concurrency."
)


def _resource_pressure_payload() -> dict:
"""Return a resource-pressure advisory when the blocking pool is saturated.

Sampled around a Graph Chat turn so the UI can nudge the user toward a
larger Databricks App instance instead of silently appearing to hang.
Never raises - pressure detection must not break the chat response.
"""
try:
from back.core.helpers import get_blocking_pool_stats

stats = get_blocking_pool_stats()
except Exception: # noqa: BLE001
return {"resource_pressure": False}
if stats.get("saturated"):
return {
"resource_pressure": True,
"resource_advice": _UPGRADE_INSTANCE_ADVICE,
"pool_stats": stats,
}
return {"resource_pressure": False}


def _chat_cache(session_mgr: SessionManager) -> dict:
"""Return the Graph Chat session cache, creating an empty one if absent."""
Expand Down Expand Up @@ -2098,6 +2129,7 @@ async def dtwin_assistant_chat(
"tools": tool_calls,
"iterations": agent_result.iterations,
"usage": agent_result.usage,
**_resource_pressure_payload(),
}


Expand Down Expand Up @@ -2251,6 +2283,7 @@ async def _generate():
"iterations": agent_result.iterations,
"usage": agent_result.usage,
"success": agent_result.success,
**_resource_pressure_payload(),
}) + "\n\n"
break

Expand Down Expand Up @@ -2481,12 +2514,13 @@ async def dtwin_triples_find(
published as a version.
"""
from back.core.helpers import sql_escape
from back.core.query_limits import get_graph_chat_result_cap

if not entity_type and not search:
raise ValidationError("Provide at least entity_type or search")

depth = max(1, min(int(depth or 1), 10))
limit = max(1, min(int(limit or 1000), 10000))
limit = max(1, min(int(limit or 1000), get_graph_chat_result_cap()))
offset = max(0, int(offset or 0))

domain = get_domain(session_mgr)
Expand Down
Loading