Skip to content

Commit 0365442

Browse files
authored
Fix memory leak for ctx.run for long running invocation (#755)
1 parent 96272c6 commit 0365442

4 files changed

Lines changed: 312 additions & 0 deletions

File tree

packages/libs/restate-sdk/src/context_impl.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1039,6 +1039,7 @@ export class RunClosuresTracker {
10391039
if (runClosure === undefined) {
10401040
throw new Error(`Handle ${handle} doesn't exist`);
10411041
}
1042+
this.runsToExecute.delete(handle);
10421043
runClosure()
10431044
.finally(() => this.channel.signal())
10441045
.catch(() => {});

packages/tests/restate-e2e-services/src/memory_leak.ts

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,37 @@ export interface MemoryLoadInput {
1616
payloadBytes?: number;
1717
}
1818

19+
export interface RunClosureRetentionInput {
20+
runCount?: number;
21+
payloadBytes?: number;
22+
holdMillis?: number;
23+
}
24+
25+
export interface RunClosureRetentionReleaseInput {
26+
invocationId?: string;
27+
}
28+
29+
export type RunClosureRetentionPhase =
30+
| "idle"
31+
| "capturing"
32+
| "holding"
33+
| "released"
34+
| "completed";
35+
36+
export interface RunClosureRetentionStatus {
37+
invocationId?: string;
38+
phase: RunClosureRetentionPhase;
39+
completedRuns: number;
40+
runCount: number;
41+
payloadBytes: number;
42+
}
43+
44+
export interface RunClosureRetentionResult {
45+
invocationId: string;
46+
completedRuns: number;
47+
payloadBytes: number;
48+
}
49+
1950
export interface MemoryStatsInput {
2051
forceGc?: boolean;
2152
}
@@ -41,10 +72,54 @@ function boundedPayloadBytes(input: MemoryLoadInput | undefined): number {
4172
return Math.max(0, Math.min(input?.payloadBytes ?? 512, 64 * 1024));
4273
}
4374

75+
function boundedRunClosureRetentionPayloadBytes(
76+
input: RunClosureRetentionInput | undefined
77+
): number {
78+
return Math.max(0, Math.min(input?.payloadBytes ?? 128 * 1024, 512 * 1024));
79+
}
80+
81+
function boundedRunClosureRetentionRunCount(
82+
input: RunClosureRetentionInput | undefined
83+
): number {
84+
return Math.max(1, Math.min(input?.runCount ?? 200, 2_000));
85+
}
86+
87+
function boundedRunClosureRetentionHoldMillis(
88+
input: RunClosureRetentionInput | undefined
89+
): number {
90+
return Math.max(1, Math.min(input?.holdMillis ?? 30_000, 120_000));
91+
}
92+
4493
function allocatePayload(input: MemoryLoadInput | undefined): number {
4594
return "x".repeat(boundedPayloadBytes(input)).length;
4695
}
4796

97+
function allocateRunClosurePayload(
98+
runIndex: number,
99+
payloadBytes: number
100+
): number[] {
101+
const elementCount = Math.max(1, Math.ceil(payloadBytes / 8));
102+
const payload = new Array<number>(elementCount);
103+
104+
for (let elementIndex = 0; elementIndex < elementCount; elementIndex++) {
105+
payload[elementIndex] = runIndex + elementIndex + 0.5;
106+
}
107+
108+
return payload;
109+
}
110+
111+
async function completeRunWithCapturedPayload(
112+
ctx: restate.Context,
113+
runIndex: number,
114+
payloadBytes: number
115+
): Promise<number> {
116+
const capturedPayload = allocateRunClosurePayload(runIndex, payloadBytes);
117+
return ctx.run(
118+
`run-closure-retention-captured-payload-${runIndex}`,
119+
() => capturedPayload.length
120+
);
121+
}
122+
48123
function forceGcIfAvailable(): boolean {
49124
const gc = (globalThis as { gc?: () => void }).gc;
50125
if (!gc) return false;
@@ -83,6 +158,41 @@ const passThroughHooks: HooksProvider = () => ({
83158
},
84159
});
85160

161+
const runClosureRetentionStatus: RunClosureRetentionStatus = {
162+
phase: "idle",
163+
completedRuns: 0,
164+
runCount: 0,
165+
payloadBytes: 0,
166+
};
167+
let releaseRunClosureRetentionHold: (() => void) | undefined;
168+
169+
function updateRunClosureRetentionStatus(
170+
update: Partial<RunClosureRetentionStatus>
171+
): RunClosureRetentionStatus {
172+
Object.assign(runClosureRetentionStatus, update);
173+
return { ...runClosureRetentionStatus };
174+
}
175+
176+
function waitForRunClosureRetentionRelease(holdMillis: number): Promise<void> {
177+
return new Promise<void>((resolve) => {
178+
let settled = false;
179+
const timeout = setTimeout(release, holdMillis);
180+
181+
function release() {
182+
if (settled) return;
183+
184+
settled = true;
185+
clearTimeout(timeout);
186+
if (releaseRunClosureRetentionHold === release) {
187+
releaseRunClosureRetentionHold = undefined;
188+
}
189+
resolve();
190+
}
191+
192+
releaseRunClosureRetentionHold = release;
193+
});
194+
}
195+
86196
function createMemoryLeakProbe(name: string) {
87197
return restate.service({
88198
name,
@@ -167,6 +277,90 @@ function createMemoryLeakProbe(name: string) {
167277
}
168278
),
169279

280+
runClosureRetention: async (
281+
ctx: restate.Context,
282+
input: RunClosureRetentionInput
283+
): Promise<RunClosureRetentionResult> => {
284+
const invocationId = ctx.request().id;
285+
const runCount = boundedRunClosureRetentionRunCount(input);
286+
const payloadBytes = boundedRunClosureRetentionPayloadBytes(input);
287+
const holdMillis = boundedRunClosureRetentionHoldMillis(input);
288+
289+
await ctx.run("run-closure-retention-start", () => {
290+
releaseRunClosureRetentionHold?.();
291+
releaseRunClosureRetentionHold = undefined;
292+
return updateRunClosureRetentionStatus({
293+
invocationId,
294+
phase: "capturing",
295+
completedRuns: 0,
296+
runCount,
297+
payloadBytes,
298+
});
299+
});
300+
301+
for (let runIndex = 0; runIndex < runCount; runIndex++) {
302+
await completeRunWithCapturedPayload(ctx, runIndex, payloadBytes);
303+
304+
const completedRuns = runIndex + 1;
305+
if (completedRuns % 25 === 0 || completedRuns === runCount) {
306+
await ctx.run(
307+
`run-closure-retention-progress-${completedRuns}`,
308+
() =>
309+
updateRunClosureRetentionStatus({
310+
completedRuns,
311+
})
312+
);
313+
}
314+
}
315+
316+
await ctx.run("run-closure-retention-holding", () =>
317+
updateRunClosureRetentionStatus({
318+
phase: "holding",
319+
})
320+
);
321+
await ctx.run("run-closure-retention-hold", () =>
322+
waitForRunClosureRetentionRelease(holdMillis)
323+
);
324+
await ctx.run("run-closure-retention-completed", () =>
325+
updateRunClosureRetentionStatus({
326+
phase: "completed",
327+
completedRuns: runCount,
328+
})
329+
);
330+
331+
return { invocationId, completedRuns: runCount, payloadBytes };
332+
},
333+
334+
runClosureRetentionStatus: async (
335+
ctx: restate.Context
336+
): Promise<RunClosureRetentionStatus> => {
337+
return ctx.run("run-closure-retention-status", () => ({
338+
...runClosureRetentionStatus,
339+
}));
340+
},
341+
342+
releaseRunClosureRetention: async (
343+
ctx: restate.Context,
344+
input: RunClosureRetentionReleaseInput
345+
): Promise<boolean> => {
346+
return ctx.run("release-run-closure-retention", () => {
347+
if (
348+
input?.invocationId !== undefined &&
349+
input.invocationId !== runClosureRetentionStatus.invocationId
350+
) {
351+
return false;
352+
}
353+
354+
const release = releaseRunClosureRetentionHold;
355+
if (release === undefined) return false;
356+
357+
releaseRunClosureRetentionHold = undefined;
358+
updateRunClosureRetentionStatus({ phase: "released" });
359+
release();
360+
return true;
361+
});
362+
},
363+
170364
abortTimeoutZero: restate.createServiceHandler(
171365
{
172366
inactivityTimeout: 0,

packages/tests/restate-e2e-services/test/memory_leak.test.ts

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ type InvocationGroups = Record<
3838
string[]
3939
>;
4040

41+
function formatBytes(bytes: number): string {
42+
return `${(bytes / 1024 / 1024).toFixed(1)} MiB`;
43+
}
44+
4145
describe("SDK memory pressure", { timeout: testTimeout }, () => {
4246
it("does not retain SDK heap after mixed invocation load", async () => {
4347
const ingress = clients.connect({ url: getIngressUrl() });
@@ -207,4 +211,112 @@ describe("SDK memory pressure", { timeout: testTimeout }, () => {
207211
expect.fail(report);
208212
}
209213
});
214+
215+
it("does not retain completed ctx.run closures while an invocation stays active", async () => {
216+
const ingress = clients.connect({ url: getIngressUrl() });
217+
const config = {
218+
runCount: envInt("RESTATE_E2E_MEMORY_RUN_CLOSURE_RUNS", 200),
219+
payloadBytes: envInt(
220+
"RESTATE_E2E_MEMORY_RUN_CLOSURE_PAYLOAD_BYTES",
221+
160 * 1024
222+
),
223+
holdMillis: envInt("RESTATE_E2E_MEMORY_RUN_CLOSURE_HOLD_MS", 60_000),
224+
waitTimeout: envInt("RESTATE_E2E_MEMORY_WAIT_TIMEOUT_MS", 90_000),
225+
cleanupDelay: envInt("RESTATE_E2E_MEMORY_CLEANUP_DELAY_MS", 1_000),
226+
maxHeapDeltaBytes: envInt(
227+
"RESTATE_E2E_MEMORY_RUN_CLOSURE_MAX_HEAP_DELTA_BYTES",
228+
16 * 1024 * 1024
229+
),
230+
};
231+
const client = ingress.serviceClient(memoryLeakProbe);
232+
const sendClient = ingress.serviceSendClient(
233+
memoryLeakProbe
234+
) as unknown as MemoryLeakSendClient;
235+
const baseline = await client.memoryStats({ forceGc: true });
236+
let invocationId: string | undefined;
237+
let heapWhileHolding: number | undefined;
238+
239+
expect(
240+
baseline.gcAvailable,
241+
"MemoryLeakProbe must run under node --expose-gc"
242+
).toBe(true);
243+
244+
try {
245+
const send = await sendClient.runClosureRetention({
246+
runCount: config.runCount,
247+
payloadBytes: config.payloadBytes,
248+
holdMillis: config.holdMillis,
249+
});
250+
invocationId = await send.invocationId;
251+
252+
await expect
253+
.poll(
254+
async () => {
255+
const status = await client.runClosureRetentionStatus();
256+
if (status.invocationId !== invocationId) {
257+
return { phase: "idle", completedRuns: 0 };
258+
}
259+
260+
return {
261+
phase: status.phase,
262+
completedRuns: status.completedRuns,
263+
};
264+
},
265+
{ timeout: config.waitTimeout, interval: 250 }
266+
)
267+
.toMatchObject({
268+
phase: "holding",
269+
completedRuns: config.runCount,
270+
});
271+
272+
heapWhileHolding = (
273+
await client.memoryStats({
274+
forceGc: true,
275+
})
276+
).heapUsed;
277+
} finally {
278+
if (invocationId !== undefined) {
279+
await client
280+
.releaseRunClosureRetention({ invocationId })
281+
.catch(() => undefined);
282+
}
283+
}
284+
285+
await expect
286+
.poll(
287+
async () => {
288+
const status = await client.runClosureRetentionStatus();
289+
return status.invocationId === invocationId ? status.phase : "idle";
290+
},
291+
{ timeout: config.waitTimeout, interval: 250 }
292+
)
293+
.toBe("completed");
294+
await delay(config.cleanupDelay);
295+
296+
const afterCompletion = await client.memoryStats({ forceGc: true });
297+
const heapDeltaWhileHolding =
298+
(heapWhileHolding ?? baseline.heapUsed) - baseline.heapUsed;
299+
const heapDeltaAfterCompletion =
300+
afterCompletion.heapUsed - baseline.heapUsed;
301+
const exceededThreshold =
302+
Math.max(0, heapDeltaWhileHolding) > config.maxHeapDeltaBytes ||
303+
Math.max(0, heapDeltaAfterCompletion) > config.maxHeapDeltaBytes;
304+
const report = [
305+
"ctx.run closure retention memory check",
306+
`runs: ${config.runCount}`,
307+
`payload per run: ${formatBytes(config.payloadBytes)}`,
308+
`baseline heap: ${formatBytes(baseline.heapUsed)}`,
309+
`heap while invocation is active: ${formatBytes(heapWhileHolding ?? 0)}`,
310+
`delta while active: ${formatBytes(heapDeltaWhileHolding)}`,
311+
`heap after completion: ${formatBytes(afterCompletion.heapUsed)}`,
312+
`delta after completion: ${formatBytes(heapDeltaAfterCompletion)}`,
313+
`retained heap threshold: ${formatBytes(config.maxHeapDeltaBytes)}`,
314+
].join("\n");
315+
316+
console.log(`\n${report}`);
317+
318+
if (exceededThreshold) {
319+
expect.fail(report);
320+
}
321+
});
210322
});

packages/tests/restate-e2e-services/test/memory_leak_utils.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import type * as clients from "@restatedev/restate-sdk-clients";
1111
import type {
1212
MemoryInvocationResult,
1313
MemoryLoadInput,
14+
RunClosureRetentionInput,
15+
RunClosureRetentionResult,
1416
} from "../src/memory_leak.js";
1517

1618
export interface InvocationStatusCounts {
@@ -65,6 +67,9 @@ export interface MemoryLeakSendClient {
6567
hookAndRunHook(
6668
input: MemoryLoadInput
6769
): Promise<clients.Send<MemoryInvocationResult>>;
70+
runClosureRetention(
71+
input: RunClosureRetentionInput
72+
): Promise<clients.Send<RunClosureRetentionResult>>;
6873
abortTimeoutZero(input: MemoryLoadInput): Promise<clients.Send<void>>;
6974
}
7075

0 commit comments

Comments
 (0)