From 27c32057ebd19acad465a6ff2cd74df18874fee1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 18:24:08 +0000 Subject: [PATCH] fix(clickhouse): add execution time cap to task run metrics queries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getTaskActivity` and `getAverageDurations` were registered without any `clickhouseSettings`, so neither had a server-side `max_execution_time`. Their sibling `existsQueryBuilder` in the same object literal already passes one. With no server cap, the only thing bounding a slow query is the client's `request_timeout`. That aborts the HTTP request rather than the query, so the failure surfaces as an untyped socket timeout instead of a ClickHouse timeout error, and stopping the query itself depends on the server noticing the disconnect. Set `max_execution_time: 25` on both. `@clickhouse/client` defaults `request_timeout` to 30000ms and nothing in this repo overrides it, so 25s sits below the client timeout — ClickHouse terminates the query and returns a typed error before the client gives up. Mirroring the neighbouring `10` would instead start failing queries that currently succeed in the 10-30s range. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PtQ34usZSzrSALPVBmoLYA --- .server-changes/task-run-metrics-query-time-limit.md | 6 ++++++ internal-packages/clickhouse/src/index.ts | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 .server-changes/task-run-metrics-query-time-limit.md diff --git a/.server-changes/task-run-metrics-query-time-limit.md b/.server-changes/task-run-metrics-query-time-limit.md new file mode 100644 index 00000000000..3001bed214f --- /dev/null +++ b/.server-changes/task-run-metrics-query-time-limit.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +Task metrics that take too long to load now stop with a clear error instead of hanging until the request gives up diff --git a/internal-packages/clickhouse/src/index.ts b/internal-packages/clickhouse/src/index.ts index bc8b6e662c8..bf9ad142ec8 100644 --- a/internal-packages/clickhouse/src/index.ts +++ b/internal-packages/clickhouse/src/index.ts @@ -232,10 +232,16 @@ export class ClickHouse { existsQueryBuilder: getTaskRunExistsQueryBuilder(this.reader, { max_execution_time: 10 }), tagQueryBuilder: getTaskRunTagsQueryBuilder(this.reader), pendingVersionIdsQueryBuilder: getPendingVersionIdsQueryBuilder(this.reader), - getTaskActivity: getTaskActivityQueryBuilder(this.reader), + // Cap these server-side. The client's `request_timeout` defaults to 30s, and + // without a `max_execution_time` that timeout is the only thing that stops a + // slow query — it aborts the HTTP request rather than the query, so the + // failure arrives as an untyped socket timeout. 25s keeps every query that + // currently succeeds while letting ClickHouse terminate its own work first + // and return a proper timeout error. + getTaskActivity: getTaskActivityQueryBuilder(this.reader, { max_execution_time: 25 }), getCurrentRunningStats: getCurrentRunningStats(this.reader), getChildRunStatusCounts: getChildRunStatusCounts(this.reader), - getAverageDurations: getAverageDurations(this.reader), + getAverageDurations: getAverageDurations(this.reader, { max_execution_time: 25 }), getTaskUsageByOrganization: getTaskUsageByOrganization(this.reader), }; }