Skip to content

Commit 6a835fa

Browse files
committed
fix(webapp): keep the runs-list region filter in WHERE, not PREWHERE
The region filter uses if(region != "", region, worker_queue): a run is region="" at trigger (so the expression yields worker_queue) and gets a real region at dequeue, so the expression flips from one non-empty value to another across a run versions. Under PREWHERE that is evaluated before FINAL reconciles versions, so it could keep a stale pre-dequeue version and drop the winner, returning runs whose current region no longer matches (and listRunIds drives bulk actions). Moved it back to WHERE (post-FINAL) with a regression test.
1 parent 3d7ef3b commit 6a835fa

2 files changed

Lines changed: 52 additions & 10 deletions

File tree

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -416,10 +416,13 @@ export class ClickHouseRunsRepository implements IRunsRepository {
416416
*
417417
* Immutable / additive-only columns go in PREWHERE so ClickHouse filters (and, for `tags`, uses
418418
* the skip index) before FINAL reconciles versions and before materialising the wide columns,
419-
* which is what bounds memory on these scans. `status` is the one lifecycle-mutable filter, so it
420-
* stays in WHERE (post-FINAL): PREWHERE-ing it would keep a stale version and drop the winner. The
421-
* `(organization_id, project_id, environment_id)` primary-key prefix and the `created_at` range
422-
* stay in WHERE so they keep driving primary-key and partition pruning.
419+
* which is what bounds memory on these scans. A filter stays in WHERE (post-FINAL) when its truth
420+
* value can flip across a run's versions, since PREWHERE could then keep a stale version and drop
421+
* the winner: `status` (lifecycle-mutable), and `regions` (its `if(region != '', region,
422+
* worker_queue)` expression yields the worker_queue before dequeue and the region after, two
423+
* different non-empty values). The `(organization_id, project_id, environment_id)` primary-key
424+
* prefix and the `created_at` range stay in WHERE so they keep driving primary-key and partition
425+
* pruning.
423426
*/
424427
function applyRunFiltersToQueryBuilder<T>(
425428
queryBuilder: ClickhouseQueryBuilder<T>,
@@ -440,6 +443,12 @@ function applyRunFiltersToQueryBuilder<T>(
440443
queryBuilder.where("status IN {statuses: Array(String)}", { statuses: options.statuses });
441444
}
442445

446+
if (options.regions && options.regions.length > 0) {
447+
queryBuilder.where("if(region != '', region, worker_queue) IN {regions: Array(String)}", {
448+
regions: options.regions,
449+
});
450+
}
451+
443452
// Period is a number of milliseconds duration
444453
if (options.period) {
445454
queryBuilder.where("created_at >= fromUnixTimestamp64Milli({period: Int64})", {
@@ -508,12 +517,6 @@ function applyRunFiltersToQueryBuilder<T>(
508517
queryBuilder.prewhere("queue IN {queues: Array(String)}", { queues: options.queues });
509518
}
510519

511-
if (options.regions && options.regions.length > 0) {
512-
queryBuilder.prewhere("if(region != '', region, worker_queue) IN {regions: Array(String)}", {
513-
regions: options.regions,
514-
});
515-
}
516-
517520
if (options.machines && options.machines.length > 0) {
518521
queryBuilder.prewhere("machine_preset IN {machines: Array(String)}", {
519522
machines: options.machines,

apps/webapp/test/runsListQueryShape.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,43 @@ describe("runs list query shape (PREWHERE routing under FINAL)", () => {
7474
expect(runIds.sort()).toEqual([pendingOld.id, pendingRecent.id].sort());
7575
}
7676
);
77+
78+
containerTest(
79+
"region filter uses the post-FINAL effective region, not a pre-dequeue worker_queue version",
80+
async ({ clickhouseContainer, prisma }) => {
81+
const clickhouse = new ClickHouse({
82+
url: clickhouseContainer.getConnectionUrl(),
83+
name: "query-shape-region-test",
84+
});
85+
86+
const ctx = await seedParents(prisma, "region");
87+
const run = await createRun(prisma, ctx, { friendlyId: "run_region" });
88+
89+
const shared = {
90+
taskIdentifier: "webhook.deliver",
91+
workerQueue: "wq-legacy",
92+
createdAt: new Date(Date.now() - 1 * DAY_MS),
93+
};
94+
95+
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) },
98+
]);
99+
100+
const repository = new RunsRepository({ prisma, clickhouse });
101+
const listArgs = {
102+
page: { size: 10 } as const,
103+
period: "365d",
104+
organizationId: ctx.organizationId,
105+
projectId: ctx.projectId,
106+
environmentId: ctx.environmentId,
107+
};
108+
109+
const byRegion = await repository.listRunIds({ ...listArgs, regions: ["us-east-1"] });
110+
expect(byRegion.runIds).toEqual([run.id]);
111+
112+
const byWorkerQueue = await repository.listRunIds({ ...listArgs, regions: ["wq-legacy"] });
113+
expect(byWorkerQueue.runIds).toEqual([]);
114+
}
115+
);
77116
});

0 commit comments

Comments
 (0)