Skip to content

Commit c1cd1af

Browse files
committed
fix(webapp): move region and error_fingerprint filters to PREWHERE
Both are set once and never change across a run version history, so they are safe in PREWHERE (evaluated before FINAL): error_fingerprint is empty until a terminal error status and then fixed (those statuses are final, so it is never cleared), and region is set once at dequeue and never reassigned. The region filter also drops the worker_queue fallback and matches on region directly, since region is populated for effectively all non-development runs and the fallback only affected not-yet-dequeued runs (which have not run in any region yet). status and machine_preset stay in WHERE because they genuinely change as a run runs (lifecycle, and OOM machine escalation).
1 parent fcd9a8d commit c1cd1af

2 files changed

Lines changed: 36 additions & 31 deletions

File tree

apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -416,17 +416,16 @@ export class ClickHouseRunsRepository implements IRunsRepository {
416416
*
417417
* A filter may go in PREWHERE only if its truth value can never flip true->false across a run's
418418
* versions, because PREWHERE is evaluated before FINAL reconciles versions and would otherwise keep
419-
* a stale matching version and drop the winning one. That holds for trigger-time identity columns
420-
* that never change (task_identifier, task_version, schedule_id, is_test, root_run_id, batch_id,
421-
* friendly_id, queue, task_kind) and for append-only arrays under `hasAny`/`hasAll` (tags,
422-
* bulk_action_group_ids), so those go in PREWHERE to filter (and, for tags, use the skip index)
423-
* before FINAL and before materialising the wide columns, which is what bounds memory on these
424-
* scans. Columns that reflect execution/outcome and change as a run runs stay in WHERE (post-FINAL):
425-
* `status`, `machine_preset` (can escalate on OOM retry), `error_fingerprint` (set/cleared with
426-
* status), and the `regions` expression (`if(region != '', region, worker_queue)` yields the
427-
* worker_queue before dequeue and the region after). The `(organization_id, project_id,
428-
* environment_id)` primary-key prefix and the `created_at` range also stay in WHERE so they keep
429-
* driving primary-key and partition pruning.
419+
* a stale matching version and drop the winning one. That holds for columns that are only ever set
420+
* once and never change: trigger-time identity columns (task_identifier, task_version, schedule_id,
421+
* is_test, root_run_id, batch_id, friendly_id, queue, task_kind), append-only arrays under
422+
* `hasAny`/`hasAll` (tags, bulk_action_group_ids), `region` (set once at dequeue, `''` -> value,
423+
* never changes), and `error_fingerprint` (empty until a terminal error status, then fixed). Those
424+
* go in PREWHERE to filter (and, for tags, use the skip index) before FINAL and before materialising
425+
* the wide columns, which is what bounds memory on these scans. Columns that change as a run runs
426+
* stay in WHERE (post-FINAL): `status` (lifecycle) and `machine_preset` (escalates on OOM retry).
427+
* The `(organization_id, project_id, environment_id)` primary-key prefix and the `created_at` range
428+
* also stay in WHERE so they keep driving primary-key and partition pruning.
430429
*/
431430
function applyRunFiltersToQueryBuilder<T>(
432431
queryBuilder: ClickhouseQueryBuilder<T>,
@@ -447,24 +446,12 @@ function applyRunFiltersToQueryBuilder<T>(
447446
queryBuilder.where("status IN {statuses: Array(String)}", { statuses: options.statuses });
448447
}
449448

450-
if (options.regions && options.regions.length > 0) {
451-
queryBuilder.where("if(region != '', region, worker_queue) IN {regions: Array(String)}", {
452-
regions: options.regions,
453-
});
454-
}
455-
456449
if (options.machines && options.machines.length > 0) {
457450
queryBuilder.where("machine_preset IN {machines: Array(String)}", {
458451
machines: options.machines,
459452
});
460453
}
461454

462-
if (options.errorId) {
463-
queryBuilder.where("error_fingerprint = {errorFingerprint: String}", {
464-
errorFingerprint: ErrorId.toId(options.errorId),
465-
});
466-
}
467-
468455
// Period is a number of milliseconds duration
469456
if (options.period) {
470457
queryBuilder.where("created_at >= fromUnixTimestamp64Milli({period: Int64})", {
@@ -533,6 +520,16 @@ function applyRunFiltersToQueryBuilder<T>(
533520
queryBuilder.prewhere("queue IN {queues: Array(String)}", { queues: options.queues });
534521
}
535522

523+
if (options.regions && options.regions.length > 0) {
524+
queryBuilder.prewhere("region IN {regions: Array(String)}", { regions: options.regions });
525+
}
526+
527+
if (options.errorId) {
528+
queryBuilder.prewhere("error_fingerprint = {errorFingerprint: String}", {
529+
errorFingerprint: ErrorId.toId(options.errorId),
530+
});
531+
}
532+
536533
if (options.taskKinds && options.taskKinds.length > 0) {
537534
const includesStandard = options.taskKinds.includes("STANDARD");
538535
// Include empty string when filtering for STANDARD (default value for pre-existing runs)

apps/webapp/test/runsListQueryShape.test.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,25 +76,33 @@ describe("runs list query shape (PREWHERE routing under FINAL)", () => {
7676
);
7777

7878
containerTest(
79-
"region filter uses the post-FINAL effective region, not a pre-dequeue worker_queue version",
79+
"region filter matches a dequeued run by its region; a still-queued run is not matched",
8080
async ({ clickhouseContainer, prisma }) => {
8181
const clickhouse = new ClickHouse({
8282
url: clickhouseContainer.getConnectionUrl(),
8383
name: "query-shape-region-test",
8484
});
8585

8686
const ctx = await seedParents(prisma, "region");
87-
const run = await createRun(prisma, ctx, { friendlyId: "run_region" });
87+
const dequeued = await createRun(prisma, ctx, { friendlyId: "run_dequeued" });
88+
const queued = await createRun(prisma, ctx, { friendlyId: "run_queued" });
8889

8990
const shared = {
9091
taskIdentifier: "webhook.deliver",
91-
workerQueue: "wq-legacy",
9292
createdAt: new Date(Date.now() - 1 * DAY_MS),
9393
};
9494

9595
await insertTaskRunV2Rows(clickhouse, [
96-
{ ...run, ...shared, region: "", updatedAt: new Date(Date.now() - 2 * DAY_MS) },
97-
{ ...run, ...shared, region: "us-east-1", updatedAt: new Date(Date.now() - 1 * DAY_MS) },
96+
{ ...dequeued, ...shared, region: "", updatedAt: new Date(Date.now() - 2 * DAY_MS) },
97+
{
98+
...dequeued,
99+
...shared,
100+
region: "us-east-1",
101+
updatedAt: new Date(Date.now() - 1 * DAY_MS),
102+
},
103+
]);
104+
await insertTaskRunV2Rows(clickhouse, [
105+
{ ...queued, ...shared, region: "", updatedAt: new Date(Date.now() - 1 * DAY_MS) },
98106
]);
99107

100108
const repository = new RunsRepository({ prisma, clickhouse });
@@ -107,10 +115,10 @@ describe("runs list query shape (PREWHERE routing under FINAL)", () => {
107115
};
108116

109117
const byRegion = await repository.listRunIds({ ...listArgs, regions: ["us-east-1"] });
110-
expect(byRegion.runIds).toEqual([run.id]);
118+
expect(byRegion.runIds).toEqual([dequeued.id]);
111119

112-
const byWorkerQueue = await repository.listRunIds({ ...listArgs, regions: ["wq-legacy"] });
113-
expect(byWorkerQueue.runIds).toEqual([]);
120+
const otherRegion = await repository.listRunIds({ ...listArgs, regions: ["us-west-2"] });
121+
expect(otherRegion.runIds).toEqual([]);
114122
}
115123
);
116124
});

0 commit comments

Comments
 (0)