Skip to content

Commit 1e10e5e

Browse files
committed
Merge branch 'feat/waitpoint-envelope-resolver-tri-13441' into feat/waitpoint-mint-flag-wiring-tri-13442
Fourteen commits from the base, mostly performance work on the completed-waitpoint record path. Two conflicts, both unions rather than choices. The waitpoint system now takes two injected things, not one: the base's O(1) records gate and this branch's coordinator. The base still built its own Postgres arm inline; that construction stays deleted, because the engine supplies a router over both arms instead. Nothing else in the merge touches the mint flag or the routing.
2 parents 7048097 + dbefc5e commit 1e10e5e

19 files changed

Lines changed: 1745 additions & 175 deletions

internal-packages/run-engine/src/engine/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,9 @@ export class RunEngine {
437437
resources,
438438
executionSnapshotSystem: this.executionSnapshotSystem,
439439
enqueueSystem: this.enqueueSystem,
440+
...(options.completedWaitpointRecordsEnabled && {
441+
completedWaitpointRecordsEnabled: options.completedWaitpointRecordsEnabled,
442+
}),
440443
coordinator: new WaitpointRouterCoordinator({
441444
meter: this.meter,
442445
legacy: new LegacyPostgresWaitpointCoordinator({

internal-packages/run-engine/src/engine/systems/executionSnapshotSystem.ts

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ async function getSnapshotWaitpointIdsWithPresence(
173173
* This is necessary because waitpoints can have large outputs (100KB+),
174174
* and fetching many at once can exceed Node.js string limits.
175175
*/
176-
export async function fetchWaitpointsInChunks(
176+
async function fetchWaitpointsInChunks(
177177
prisma: PrismaClientOrTransaction,
178178
waitpointIds: string[],
179179
runStore?: RunStore,
@@ -200,6 +200,67 @@ export async function fetchWaitpointsInChunks(
200200
return allWaitpoints;
201201
}
202202

203+
/**
204+
* The columns the completion envelope is built from, and only those.
205+
*
206+
* `fetchWaitpointsInChunks` reads whole rows because the snapshot hydration builds a full
207+
* executor waitpoint from them. The envelope needs fewer, so projecting drops `tags`,
208+
* `projectId`, `environmentId`, `createdAt`, `updatedAt` and `idempotencyKeyExpiresAt` from a
209+
* read that happens on the writer inside the run lock. It cannot drop `output`, which is the
210+
* 100KB+ column and also the payload the envelope exists to carry.
211+
*
212+
* `status` is here for the arm's COMPLETED filter, not for the mapper.
213+
*/
214+
const WAITPOINT_ENVELOPE_SELECT = {
215+
id: true,
216+
friendlyId: true,
217+
type: true,
218+
status: true,
219+
completedAt: true,
220+
output: true,
221+
outputType: true,
222+
outputIsError: true,
223+
completedByTaskRunId: true,
224+
completedByBatchId: true,
225+
completedAfter: true,
226+
idempotencyKey: true,
227+
userProvidedIdempotencyKey: true,
228+
inactiveIdempotencyKey: true,
229+
} satisfies Prisma.WaitpointSelect;
230+
231+
export type WaitpointEnvelopeRow = Pick<Waitpoint, keyof typeof WAITPOINT_ENVELOPE_SELECT>;
232+
233+
/**
234+
* The projected sibling of `fetchWaitpointsInChunks`, for the envelope read.
235+
*
236+
* Chunked identically, and for the same reason: a waitpoint output can be 100KB+, so a large
237+
* fan-in read whole can exceed Node's string limits. `boundedIn` pads for plan-cache stability,
238+
* it does not bound the set. `runId` is the routing hint the router needs to read the run's own
239+
* store instead of fanning every chunk across both run-ops databases.
240+
*/
241+
export async function fetchWaitpointEnvelopeRowsInChunks(
242+
prisma: PrismaClientOrTransaction,
243+
waitpointIds: string[],
244+
runStore?: RunStore,
245+
runId?: string
246+
): Promise<WaitpointEnvelopeRow[]> {
247+
if (waitpointIds.length === 0) return [];
248+
249+
const rows: WaitpointEnvelopeRow[] = [];
250+
for (let i = 0; i < waitpointIds.length; i += WAITPOINT_CHUNK_SIZE) {
251+
const chunk = waitpointIds.slice(i, i + WAITPOINT_CHUNK_SIZE);
252+
const args = {
253+
where: { id: { in: boundedIn(chunk) } },
254+
select: WAITPOINT_ENVELOPE_SELECT,
255+
};
256+
const found = runStore
257+
? await runStore.findManyWaitpoints(args, prisma, runId)
258+
: await prisma.waitpoint.findMany(args);
259+
rows.push(...found);
260+
}
261+
return rows;
262+
}
263+
203264
/**
204265
* Gets the most recent valid snapshot for a run. When `environmentId` is provided the read is scoped
205266
* to that environment (tenant boundary): a run in another environment reads as not-found and rejects

internal-packages/run-engine/src/engine/systems/waitpointSystem.ts

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,29 @@ export type WaitpointSystemOptions = {
3030
enqueueSystem: EnqueueSystem;
3131
/** Which coordinator owns waitpoint state. The engine supplies a router over both arms. */
3232
coordinator: WaitpointCoordinator;
33+
/**
34+
* Whether this run's snapshots can hold a completed-waitpoint record set. Must be O(1).
35+
*
36+
* Sits in FRONT of the id-format scan so a run that cannot hold records pays one predicate
37+
* call, not one `parseWaitpointId` per blocking waitpoint. It has to be injected rather than
38+
* derived here: the answer is per-organisation, since it follows the snapshot store's own
39+
* rollout, and the store keeps that state private to itself.
40+
*
41+
* Takes the organisation id, not just the run id. The decision is organisation-scoped, and an
42+
* opaque run id cannot answer it without a lookup -- which would put a read back on the path
43+
* this gate exists to keep free. Both call sites already hold it on the snapshot they are
44+
* transitioning from, so it costs nothing to pass. The run id rides along for logging and for
45+
* any future per-run override.
46+
*
47+
* Positional rather than an options object, deliberately: an object literal here would be
48+
* allocated on every resume, including the ones that exist only to be told no. Two arguments
49+
* make the disabled path genuinely free rather than nearly free.
50+
*
51+
* Defaults to never. A record set is only reachable through the snapshot store, so until the
52+
* ticket that wires that store supplies this predicate there is no run for which one could
53+
* exist, and no resume does any of this work.
54+
*/
55+
completedWaitpointRecordsEnabled?: (runId: string, organizationId: string) => boolean;
3356
};
3457

3558
type WaitpointContinuationWaitpoint = Pick<Waitpoint, "id" | "type" | "completedAfter" | "status">;
@@ -53,11 +76,13 @@ export class WaitpointSystem {
5376
private readonly executionSnapshotSystem: ExecutionSnapshotSystem;
5477
private readonly enqueueSystem: EnqueueSystem;
5578
private readonly coordinator: WaitpointCoordinator;
79+
private readonly recordsEnabled: (runId: string, organizationId: string) => boolean;
5680

5781
constructor(private readonly options: WaitpointSystemOptions) {
5882
this.$ = options.resources;
5983
this.executionSnapshotSystem = options.executionSnapshotSystem;
6084
this.enqueueSystem = options.enqueueSystem;
85+
this.recordsEnabled = options.completedWaitpointRecordsEnabled ?? (() => false);
6186
this.coordinator = options.coordinator;
6287
}
6388

@@ -624,6 +649,7 @@ export class WaitpointSystem {
624649
// appending, and they must not pay an envelope read to do it.
625650
const completedWaitpointRecords = await this.#completedWaitpointRecordsFor(
626651
runId,
652+
snapshot.organizationId,
627653
blockingWaitpoints
628654
);
629655

@@ -697,6 +723,7 @@ export class WaitpointSystem {
697723

698724
const completedWaitpointRecords = await this.#completedWaitpointRecordsFor(
699725
runId,
726+
snapshot.organizationId,
700727
blockingWaitpoints
701728
);
702729

@@ -810,18 +837,44 @@ export class WaitpointSystem {
810837
* The record set for one resume, or undefined when no blocking waitpoint carries a store-format
811838
* id.
812839
*
813-
* Gated on id FORMAT, not residency. The two are not the same during a migration: a
840+
* Gated on the record set being reachable at all, and only then on id FORMAT rather than
841+
* residency. Format and residency are not the same during a migration: a
814842
* store-format id can still be served by the Postgres arm, exactly as run-ops ids were for
815843
* runs. Whichever arm owns it answers, so the gate only decides whether to ask at all.
816844
*
817845
* That gate is what keeps this inert. `parseWaitpointId` reports legacy for every id minted
818846
* today, so no live resume reads an envelope or writes a record until a waitpoint mints in
819847
* store format.
848+
*
849+
* ROLLOUT ORDER. Because the gate reads the id and not the organisation, the first store-format
850+
* mint is what starts the cost, for every organisation that then holds one -- not the first
851+
* snapshot-store flip. The arm that answers here is the Postgres one until a store arm is
852+
* wired, so a mint enabled ahead of the store means a projected `Waitpoint` read on the
853+
* WRITER, inside the run lock, once per resume. That is bounded and correct, but it is not
854+
* free, so store-format minting should follow the snapshot store rather than lead it.
820855
*/
821856
async #completedWaitpointRecordsFor(
822857
runId: string,
858+
organizationId: string,
823859
blockingWaitpoints: RunBlockEdge[]
824860
): Promise<CompletedWaitpointRecord[] | undefined> {
861+
// O(1) first, and unconditionally first: a run whose snapshots cannot hold a record set has
862+
// nothing to build, and deciding that by walking its blocking waitpoints made every resume
863+
// for every organisation pay a scan proportional to its fan-in to reach the same answer.
864+
// Defaults to never, so today this returns here for everyone.
865+
if (!this.recordsEnabled(runId, organizationId)) {
866+
return undefined;
867+
}
868+
869+
// Only then the id-format scan, which is what keeps a MIXED cycle working once records are
870+
// enabled: an organisation mid-rollout holds waitpoints minted either side of the flip, and
871+
// the format is the only thing that says which half each id belongs to. `.some` before the
872+
// dedup so an enabled run holding no store-format id still allocates nothing.
873+
if (!blockingWaitpoints.some((b) => parseWaitpointId(b.waitpoint.id).format === "b32hexW")) {
874+
return undefined;
875+
}
876+
877+
// Only a set that really holds one pays for the dedup.
825878
const storeFormatIds = [
826879
...new Set(
827880
blockingWaitpoints
@@ -830,10 +883,6 @@ export class WaitpointSystem {
830883
),
831884
];
832885

833-
if (storeFormatIds.length === 0) {
834-
return undefined;
835-
}
836-
837886
const sources = await this.coordinator.readCompletionEnvelopes({
838887
runId,
839888
waitpointIds: storeFormatIds,

0 commit comments

Comments
 (0)