Skip to content

Commit 7f8db90

Browse files
committed
fix(webapp): use only per-query caps on the runs-list ClickHouse pool
Drops max_memory_usage_for_user and max_concurrent_queries_for_user. Those are per-ClickHouse-user limits, and every connection is the default user, so hitting the shared budget rejects whichever query arrives next rather than the one responsible, which would fail queries for uninvolved tenants. The per-query caps (max_execution_time, max_memory_usage, max_threads) bound a bad query to itself, and the server-level max_server_memory_usage protects the node.
1 parent d2bbe1b commit 7f8db90

2 files changed

Lines changed: 9 additions & 15 deletions

File tree

apps/webapp/app/env.server.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2237,8 +2237,6 @@ const EnvironmentSchema = z
22372237
RUNS_LIST_CLICKHOUSE_MAX_EXECUTION_TIME: z.coerce.number().int().default(35),
22382238
RUNS_LIST_CLICKHOUSE_MAX_THREADS: z.coerce.number().int().optional(),
22392239
RUNS_LIST_CLICKHOUSE_MAX_MEMORY_USAGE: z.coerce.number().int().optional(),
2240-
RUNS_LIST_CLICKHOUSE_MAX_MEMORY_USAGE_FOR_USER: z.coerce.number().int().optional(),
2241-
RUNS_LIST_CLICKHOUSE_MAX_CONCURRENT_QUERIES_FOR_USER: z.coerce.number().int().optional(),
22422240
RUNS_LIST_CLICKHOUSE_READONLY: z.enum(["0", "1", "2"]).default("2"),
22432241
/**
22442242
* Dedicated ClickHouse service for queue metrics: the ingestion consumer's inserts and every

apps/webapp/app/services/clickhouse/clickhouseFactory.server.ts

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -293,11 +293,15 @@ function initializeRealtimeClickhouseClient(): ClickHouse {
293293
}
294294

295295
/**
296-
* Server-side query protection for the runs-list read pool. Safe as client-level settings ONLY
297-
* because this pool is read-only (no inserts); a client-level `max_execution_time` on a mixed
298-
* read+write pool would also kill slow inserts. `readonly=2` enforces read-only while still
299-
* allowing these settings to apply (`readonly=1` rejects them). `max_concurrent_queries_for_user`
300-
* is a per-ClickHouse-user (`default`) fail-fast circuit breaker, not per-tenant isolation.
296+
* Server-side query protection for the runs-list read pool. Every setting here is PER-QUERY, so a
297+
* pathological query only ever kills itself: a slow one hits `max_execution_time`, a memory-hungry
298+
* one hits `max_memory_usage`, a thread-hungry one hits `max_threads`. Per-USER limits
299+
* (`max_*_for_user`) are deliberately NOT used: everything connects as `default`, so a per-user cap
300+
* would reject whichever query arrives once the shared budget is hit, punishing innocent tenants
301+
* for a noisy one. The node itself is protected by the server-level `max_server_memory_usage`.
302+
* Safe as client-level settings ONLY because this pool is read-only; on a mixed read+write pool a
303+
* client-level `max_execution_time` would also kill slow inserts. `readonly=2` enforces read-only
304+
* while still allowing these settings to apply (`readonly=1` rejects them).
301305
*/
302306
function getRunsListClickhouseSettings(): ClickHouseSettings {
303307
const settings: ClickHouseSettings = {
@@ -314,14 +318,6 @@ function getRunsListClickhouseSettings(): ClickHouseSettings {
314318
if (env.RUNS_LIST_CLICKHOUSE_MAX_MEMORY_USAGE !== undefined) {
315319
settings.max_memory_usage = env.RUNS_LIST_CLICKHOUSE_MAX_MEMORY_USAGE.toString();
316320
}
317-
if (env.RUNS_LIST_CLICKHOUSE_MAX_MEMORY_USAGE_FOR_USER !== undefined) {
318-
settings.max_memory_usage_for_user =
319-
env.RUNS_LIST_CLICKHOUSE_MAX_MEMORY_USAGE_FOR_USER.toString();
320-
}
321-
if (env.RUNS_LIST_CLICKHOUSE_MAX_CONCURRENT_QUERIES_FOR_USER !== undefined) {
322-
settings.max_concurrent_queries_for_user =
323-
env.RUNS_LIST_CLICKHOUSE_MAX_CONCURRENT_QUERIES_FOR_USER;
324-
}
325321

326322
return settings;
327323
}

0 commit comments

Comments
 (0)