Skip to content

Commit 0468fc4

Browse files
committed
perf(run-engine): bound the envelope read's concurrency
The store arm launched one HMGET per waitpoint through a single Promise.all, so a 1000-item batch fan-in would issue 1000 concurrent commands at once. Each is small, but the burst is the cost. Chunks at 100, matching the bound the snapshot hydration already uses for the same shape of read. Stays one command per key, so nothing can span two cluster slots and the single-slot assertion is still unnecessary here. Two tests cover the chunk boundary at 250 and a mixed complete/pending set at 150, since an increment that disagrees with the slice is the realistic failure of hand-rolled chunking and is invisible in a single-chunk test.
1 parent 3653811 commit 0468fc4

2 files changed

Lines changed: 68 additions & 3 deletions

File tree

‎internal-packages/run-engine/src/engine/waitpointCoordinator/storeCoordinator.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1973,3 +1973,53 @@ describe("readCompletionEnvelopes", () => {
19731973
}
19741974
});
19751975
});
1976+
1977+
// The read is chunked so a large fan-in cannot burst. This asserts correctness ACROSS the chunk
1978+
// boundary, which is where an off-by-one in the slice would show: a count either side of 100.
1979+
describe("readCompletionEnvelopes chunking", () => {
1980+
redisTest("returns every envelope across a chunk boundary", async ({ redisOptions }) => {
1981+
const store = coordinator(redisOptions);
1982+
try {
1983+
const ids = Array.from({ length: 250 }, (_, i) => `w_chunk_${i}`);
1984+
for (const id of ids) {
1985+
await store.createIfAbsent({ record: record(id), status: "PENDING" });
1986+
await store.complete({ waitpointId: id, completion: completion() });
1987+
}
1988+
1989+
const envelopes = await store.readCompletionEnvelopes({
1990+
runId: "run_chunked",
1991+
waitpointIds: ids,
1992+
});
1993+
1994+
expect(envelopes).toHaveLength(250);
1995+
expect(new Set(envelopes.map((e) => e.id)).size).toBe(250);
1996+
} finally {
1997+
await store.quit();
1998+
}
1999+
});
2000+
2001+
redisTest("omits only the incomplete ones in a mixed chunked read", async ({ redisOptions }) => {
2002+
const store = coordinator(redisOptions);
2003+
try {
2004+
const ids = Array.from({ length: 150 }, (_, i) => `w_mix_${i}`);
2005+
for (const [i, id] of ids.entries()) {
2006+
await store.createIfAbsent({ record: record(id), status: "PENDING" });
2007+
// Leave every third pending, including ids either side of the chunk boundary.
2008+
if (i % 3 !== 0) {
2009+
await store.complete({ waitpointId: id, completion: completion() });
2010+
}
2011+
}
2012+
2013+
const envelopes = await store.readCompletionEnvelopes({
2014+
runId: "run_chunked",
2015+
waitpointIds: ids,
2016+
});
2017+
2018+
const expected = ids.filter((_, i) => i % 3 !== 0).length;
2019+
expect(envelopes).toHaveLength(expected);
2020+
expect(envelopes.every((e) => e.id.startsWith("w_mix_"))).toBe(true);
2021+
} finally {
2022+
await store.quit();
2023+
}
2024+
});
2025+
});

‎internal-packages/run-engine/src/engine/waitpointCoordinator/storeCoordinator.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ function parseJson<T>(raw: string | undefined): T | undefined {
163163
return raw ? (JSON.parse(raw) as T) : undefined;
164164
}
165165

166+
/**
167+
* How many envelope reads run concurrently. Matches the chunk the snapshot hydration uses for the
168+
* same shape of read, so a large fan-in bounds Redis client pressure instead of bursting.
169+
*/
170+
const ENVELOPE_READ_CHUNK_SIZE = 100;
171+
166172
export class WaitpointStoreCoordinator {
167173
private readonly redis: Redis;
168174
private readonly logger: Logger;
@@ -527,9 +533,18 @@ export class WaitpointStoreCoordinator {
527533
return [];
528534
}
529535

530-
const halves = await Promise.all(
531-
waitpointIds.map((id) => this.redis.hmget(waitpointKeys(id).record, "r", "c"))
532-
);
536+
// Chunked, not one Promise.all over the whole set. A 1000-item batch fan-in would otherwise
537+
// launch 1000 concurrent HMGETs at once, and the burst is the cost even though each command
538+
// is small. The bound mirrors the snapshot hydration's own WAITPOINT_CHUNK_SIZE, and stays
539+
// per-command so nothing can span two cluster slots.
540+
const halves: (string | null)[][] = [];
541+
for (let i = 0; i < waitpointIds.length; i += ENVELOPE_READ_CHUNK_SIZE) {
542+
const chunk = waitpointIds.slice(i, i + ENVELOPE_READ_CHUNK_SIZE);
543+
const settled = await Promise.all(
544+
chunk.map((id) => this.redis.hmget(waitpointKeys(id).record, "r", "c"))
545+
);
546+
halves.push(...settled);
547+
}
533548

534549
const out: CompletionEnvelopeSource[] = [];
535550

0 commit comments

Comments
 (0)