Skip to content

Commit 8219721

Browse files
authored
fix(run-store,webapp): correct split-database read routing, write residency, and batches list ordering (#4272)
## Summary Correctness and performance fixes for deployments that split run data across more than one database. Single-database / self-hosted deployments are unaffected (they collapse to a single read/write path). - **Batches list (dashboard):** for some organizations the Batches list could hide older batches or show them out of order. It now orders and paginates by creation time (with the id as a stable tiebreak), so every batch appears exactly once, newest first. The pagination cursor format changes; older in-flight cursors simply restart from the first page. - **Reads:** waitpoint and snapshot lookups that are keyed by a single run now read only the database that holds that run instead of querying both, removing redundant queries on hot paths (unblock, snapshot reads). - **Writes:** environment-scoped writes with no owning run (standalone wait tokens, waitpoint tags, idempotency-key resets) now land in the same database as that environment's runs, rather than defaulting to the other one. An idempotency-key reset also falls back to the other database when it matches nothing, so a reset still clears the key wherever the run actually lives. ## Notes Verified end-to-end against multi-database setups: run-keyed reads and env-scoped writes land on the correct database with no cross-database writes, and the batches list surfaces every batch in creation order. New tests cover the batches ordering/reachability and the write-residency routing.
1 parent 0ff0abd commit 8219721

16 files changed

Lines changed: 1265 additions & 134 deletions

apps/webapp/app/models/waitpointTag.server.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,30 @@ export async function createWaitpointTag({
88
tag,
99
environmentId,
1010
projectId,
11+
residency,
1112
}: {
1213
tag: string;
1314
environmentId: string;
1415
projectId: string;
16+
// Residency from the env mint kind: a tag has no owning run, so a minted-new env pins it to NEW
17+
// instead of defaulting to the draining legacy DB.
18+
residency?: "NEW" | "LEGACY";
1519
}) {
1620
if (tag.trim().length === 0) return;
1721

1822
let attempts = 0;
1923

2024
while (attempts < MAX_RETRIES) {
2125
try {
22-
return await runStore.upsertWaitpointTag({
23-
environmentId,
24-
name: tag,
25-
projectId,
26-
});
26+
return await runStore.upsertWaitpointTag(
27+
{
28+
environmentId,
29+
name: tag,
30+
projectId,
31+
},
32+
undefined,
33+
residency
34+
);
2735
} catch (error) {
2836
if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2002") {
2937
// Handle unique constraint violation (conflict)

apps/webapp/app/presenters/v3/BatchListPresenter.server.ts

Lines changed: 69 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,28 @@ type BatchRow = {
4343
batchVersion: string;
4444
};
4545

46+
// Composite keyset cursor "<createdAt-epoch-ms>_<id>". Ordering is by createdAt then id: a batch id is
47+
// a cuid (legacy) OR a run-ops id (new), and the two schemes occupy different lexical ranges, so `id`
48+
// alone is not a valid chronological order across the residency split. `id` is the stable tiebreak.
49+
// Old plain-id cursors (no "_") decode to undefined and restart from page 1 (self-healing).
50+
type BatchCursor = { createdAt: Date; id: string };
51+
function encodeBatchCursor(row: BatchCursor): string {
52+
return `${row.createdAt.getTime()}_${row.id}`;
53+
}
54+
function decodeBatchCursor(cursor: string | undefined): BatchCursor | undefined {
55+
if (!cursor) return undefined;
56+
const sep = cursor.indexOf("_");
57+
if (sep === -1) return undefined;
58+
const ms = Number(cursor.slice(0, sep));
59+
const id = cursor.slice(sep + 1);
60+
// Number.isFinite accepts e.g. 1e20, but new Date(1e20) is Invalid Date — reject it so a malformed
61+
// URL cursor self-heals to page 1 instead of reaching Prisma with an invalid date.
62+
const createdAt = new Date(ms);
63+
if (!Number.isFinite(ms) || Number.isNaN(createdAt.getTime()) || id.length === 0)
64+
return undefined;
65+
return { createdAt, id };
66+
}
67+
4668
export class BatchListPresenter extends BasePresenter {
4769
// Optional run-ops read-routing. Omitted (single-DB / self-host) => everything
4870
// reads from `_replica` exactly as today (passthrough). Field names are local to
@@ -86,17 +108,16 @@ export class BatchListPresenter extends BasePresenter {
86108
return scan(passthrough);
87109
}
88110

89-
const newRows = await scan(this.readRoute.runOpsNew ?? passthrough);
90-
91-
// New DB filled the page — skip the legacy read entirely; older rows fall on a later page.
92-
if (newRows.length >= pageSize + 1) {
93-
return newRows;
94-
}
95-
96-
const legacyRows = await scan(this.readRoute.runOpsLegacyReplica ?? passthrough);
111+
// Always read BOTH stores and merge. The old "skip legacy when new fills the page" shortcut is
112+
// unsound across the residency split: legacy cuid ids ("c…") sort ABOVE new run-ops ids ("0…")
113+
// under id order, so a new-only page can hide pre-flip legacy batches that belong ahead of it.
114+
// Ordering is by createdAt (id tiebreak), which is chronologically correct across both schemes.
115+
const [newRows, legacyRows] = await Promise.all([
116+
scan(this.readRoute.runOpsNew ?? passthrough),
117+
scan(this.readRoute.runOpsLegacyReplica ?? passthrough),
118+
]);
97119

98-
// De-dupe by id (new wins), re-sort under the page's keyset order, re-apply the over-fetch
99-
// LIMIT — reproduces the pageSize+1 window a single union scan would return.
120+
// De-dupe by id (new wins), re-sort under the page's keyset order, re-apply the over-fetch LIMIT.
100121
const byId = new Map<string, BatchRow>();
101122
for (const row of newRows) {
102123
byId.set(row.id, row);
@@ -107,10 +128,16 @@ export class BatchListPresenter extends BasePresenter {
107128
}
108129
}
109130

110-
// codepoint comparator (NEVER localeCompare): BatchTaskRun.id is ASCII (cuid or run-ops id).
111-
const sign = direction === "forward" ? 1 : -1; // forward => DESC; backward => ASC
131+
// forward => newest-first (createdAt DESC), backward => oldest-first (ASC); id is the stable
132+
// tiebreak (ASCII codepoint, NEVER localeCompare).
133+
const sign = direction === "forward" ? 1 : -1;
112134
return Array.from(byId.values())
113-
.sort((a, b) => (a.id < b.id ? sign : a.id > b.id ? -sign : 0))
135+
.sort((a, b) => {
136+
const at = a.createdAt.getTime();
137+
const bt = b.createdAt.getTime();
138+
if (at !== bt) return at < bt ? sign : -sign;
139+
return a.id < b.id ? sign : a.id > b.id ? -sign : 0;
140+
})
114141
.slice(0, pageSize + 1);
115142
}
116143

@@ -212,11 +239,28 @@ export class BatchListPresenter extends BasePresenter {
212239
}
213240
const createdAtLte: Date | undefined = time.to;
214241

242+
// Composite (createdAt, id) keyset — see encodeBatchCursor. An old plain-id cursor decodes to
243+
// undefined and restarts from page 1.
244+
const keyCursor = decodeBatchCursor(cursor);
245+
215246
const batches = await this.#scanBatchTaskRun(pageSize, direction, (client) =>
216247
client.batchTaskRun.findMany({
217248
where: {
218249
runtimeEnvironmentId: environmentId,
219-
...(cursor ? { id: direction === "forward" ? { lt: cursor } : { gt: cursor } } : {}),
250+
...(keyCursor
251+
? {
252+
OR:
253+
direction === "forward"
254+
? [
255+
{ createdAt: { lt: keyCursor.createdAt } },
256+
{ createdAt: keyCursor.createdAt, id: { lt: keyCursor.id } },
257+
]
258+
: [
259+
{ createdAt: { gt: keyCursor.createdAt } },
260+
{ createdAt: keyCursor.createdAt, id: { gt: keyCursor.id } },
261+
],
262+
}
263+
: {}),
220264
...(friendlyId ? { friendlyId } : {}),
221265
...(statuses && statuses.length > 0
222266
? { status: { in: statuses }, batchVersion: { not: "v1" } }
@@ -230,7 +274,10 @@ export class BatchListPresenter extends BasePresenter {
230274
}
231275
: {}),
232276
},
233-
orderBy: { id: direction === "forward" ? "desc" : "asc" },
277+
orderBy: [
278+
{ createdAt: direction === "forward" ? "desc" : "asc" },
279+
{ id: direction === "forward" ? "desc" : "asc" },
280+
],
234281
take: pageSize + 1,
235282
select: {
236283
id: true,
@@ -248,23 +295,24 @@ export class BatchListPresenter extends BasePresenter {
248295

249296
const hasMore = batches.length > pageSize;
250297

251-
//get cursors for next and previous pages
298+
//get cursors for next and previous pages (composite (createdAt, id) keyset)
299+
const cur = (row?: BatchRow) => (row ? encodeBatchCursor(row) : undefined);
252300
let next: string | undefined;
253301
let previous: string | undefined;
254302
switch (direction) {
255303
case "forward":
256-
previous = cursor ? batches.at(0)?.id : undefined;
304+
previous = cursor ? cur(batches.at(0)) : undefined;
257305
if (hasMore) {
258-
next = batches[pageSize - 1]?.id;
306+
next = cur(batches[pageSize - 1]);
259307
}
260308
break;
261309
case "backward":
262310
batches.reverse();
263311
if (hasMore) {
264-
previous = batches[1]?.id;
265-
next = batches[pageSize]?.id;
312+
previous = cur(batches[1]);
313+
next = cur(batches[pageSize]);
266314
} else {
267-
next = batches[pageSize - 1]?.id;
315+
next = cur(batches[pageSize - 1]);
268316
}
269317
break;
270318
}

apps/webapp/app/routes/api.v1.waitpoints.tokens.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
type PrismaClientOrTransaction,
1717
} from "~/db.server";
1818
import { type AuthenticatedEnvironment } from "~/services/apiAuth.server";
19+
import { resolveRunIdMintKind } from "~/v3/engineVersion.server";
1920
import { logger } from "~/services/logger.server";
2021
import { generateHttpCallbackUrl } from "~/services/httpCallback.server";
2122
import {
@@ -58,6 +59,16 @@ const { action } = createActionApiRoute(
5859

5960
const timeout = await parseDelay(body.timeout);
6061

62+
// A token (and its tags) has no owning run, so it can't co-locate. Resolve the env mint kind so a
63+
// minted-new env creates them on the run-ops DB (NEW) instead of defaulting to the draining LEGACY
64+
// DB by their cuid id-shape.
65+
const mintKind = await resolveRunIdMintKind({
66+
organizationId: authentication.environment.organizationId,
67+
id: authentication.environment.id,
68+
orgFeatureFlags: authentication.environment.organization.featureFlags,
69+
});
70+
const residency = mintKind === "runOpsId" ? "NEW" : "LEGACY";
71+
6172
//upsert tags
6273
let tags: { id: string; name: string }[] = [];
6374
const bodyTags = typeof body.tags === "string" ? [body.tags] : body.tags;
@@ -74,6 +85,7 @@ const { action } = createActionApiRoute(
7485
tag,
7586
environmentId: authentication.environment.id,
7687
projectId: authentication.environment.projectId,
88+
residency,
7789
});
7890
if (tagRecord) {
7991
tags.push(tagRecord);
@@ -88,6 +100,7 @@ const { action } = createActionApiRoute(
88100
idempotencyKeyExpiresAt,
89101
timeout,
90102
tags: bodyTags,
103+
standaloneResidency: residency,
91104
});
92105

93106
const $responseHeaders = await responseHeaders(authentication.environment);

apps/webapp/app/v3/services/resetIdempotencyKey.server.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,35 @@ import type { AuthenticatedEnvironment } from "~/services/apiAuth.server";
22
import { BaseService, ServiceValidationError } from "./baseService.server";
33
import { logger } from "~/services/logger.server";
44
import { getMollifierBuffer } from "~/v3/mollifier/mollifierBuffer.server";
5+
import { resolveRunIdMintKind } from "~/v3/engineVersion.server";
56

67
export class ResetIdempotencyKeyService extends BaseService {
78
public async call(
89
idempotencyKey: string,
910
taskIdentifier: string,
1011
authenticatedEnv: AuthenticatedEnvironment
1112
): Promise<{ id: string }> {
13+
// The predicate has no run id to route by. When the env mints run-ops ids its runs live on NEW,
14+
// so pin the reset to NEW and skip the wrong-DB (0-row) write to the draining legacy DB. Resolve
15+
// this only when the org (and its flags) is loaded on the env — which the authenticated API path
16+
// always provides; otherwise fall back to the two-store reset (correct, just not optimized).
17+
let residency: "NEW" | "LEGACY" = "LEGACY";
18+
if (authenticatedEnv.organization) {
19+
const mintKind = await resolveRunIdMintKind({
20+
organizationId: authenticatedEnv.organizationId,
21+
id: authenticatedEnv.id,
22+
orgFeatureFlags: authenticatedEnv.organization.featureFlags,
23+
});
24+
residency = mintKind === "runOpsId" ? "NEW" : "LEGACY";
25+
}
26+
1227
const { count: pgCount } = await this.runStore.clearIdempotencyKey(
1328
{
1429
byPredicate: {
1530
idempotencyKey,
1631
taskIdentifier,
1732
runtimeEnvironmentId: authenticatedEnv.id,
33+
residency,
1834
},
1935
},
2036
this._prisma
@@ -80,6 +96,7 @@ export class ResetIdempotencyKeyService extends BaseService {
8096
idempotencyKey,
8197
taskIdentifier,
8298
runtimeEnvironmentId: authenticatedEnv.id,
99+
residency,
83100
},
84101
},
85102
this._prisma

0 commit comments

Comments
 (0)