Skip to content

Commit cc18919

Browse files
Brooooooklynclaude
andcommitted
fix(cli/deploy): drop assetFiles refs as soon as R2 acks each PUT
streamDirectR2Deploy held every asset's bytes in `assetFiles` until finalizeDeploy returned, even for entries preflight reported as already in R2. A 142 MB mostly-cached deploy paid full resident memory for nothing. Two-stage release: - Right after preflight, prune `assetFiles` to the `needed` subset so skipped entries become GC-eligible immediately. Pre/post sizes log to the JSONL trace as `asset_cache_pruned` for retrospect. - During upload, `uploadAssetsToR2` now clears `item.body` and invokes an `onAfterPut` callback after each successful PUT. The deploy driver passes `assetFiles.delete(item.path)` so peak memory under the bounded pool tracks `R2_PUT_CONCURRENCY × asset_size`, not the full bundle. Callback is suppressed on PUT failure so callers can still re-presign / retry without losing the buffer prematurely. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a2b1964 commit cc18919

4 files changed

Lines changed: 164 additions & 5 deletions

File tree

‎packages/void/src/cli/deploy.ts‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,26 @@ async function streamDirectR2Deploy(
648648
cliLog?: CliLogger,
649649
): Promise<StreamDeployResult> {
650650
if (needed.length > 0) {
651+
// Memory: drop buffers for assets the server already has so peak
652+
// memory tracks `needed`, not the full bundle. Skipped entries become
653+
// GC-eligible immediately rather than after `finalizeDeploy` returns.
654+
// The pre-prune size is captured for the JSONL log only. We materialize
655+
// the key list before deleting so the iterator isn't disturbed mid-walk.
656+
const neededPaths = new Set(needed);
657+
const preBufferCount = assetFiles.size;
658+
const allPaths = Array.from(assetFiles.keys());
659+
for (const path of allPaths) {
660+
if (!neededPaths.has(path)) {
661+
assetFiles.delete(path);
662+
}
663+
}
664+
if (assetFiles.size !== preBufferCount) {
665+
cliLog?.info('asset_cache_pruned', {
666+
before: preBufferCount,
667+
after: assetFiles.size,
668+
});
669+
}
670+
651671
s.message(`Requesting upload URLs for ${needed.length} asset(s)...`);
652672

653673
const assetsToUpload = needed.map((path) => {
@@ -704,7 +724,12 @@ async function streamDirectR2Deploy(
704724
});
705725

706726
s.message(`Uploading ${items.length} asset(s) to R2...`);
707-
await uploadAssetsToR2(items, cliLog);
727+
// Per-PUT cleanup: once R2 acks the upload, drop the buffer from the
728+
// CLI-side cache so peak memory under the bounded upload pool stays
729+
// close to `R2_PUT_CONCURRENCY` × asset size, not the full bundle.
730+
await uploadAssetsToR2(items, cliLog, undefined, (item) => {
731+
assetFiles.delete(item.path);
732+
});
708733
} else {
709734
cliLog?.info('presign_skipped', { reason: 'no_needed_assets' });
710735
}

‎packages/void/src/cli/direct-r2-upload.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,13 @@ export interface DirectR2PutItem {
4747
path: string;
4848
/** BLAKE3 storage hash — used for log correlation and R2 key. */
4949
hash: string;
50-
/** Asset bytes. */
51-
body: Buffer;
50+
/**
51+
* Asset bytes. Cleared (set to `undefined`) by `uploadAssetsToR2` after a
52+
* successful PUT so the buffer becomes eligible for GC as soon as the
53+
* upload is acknowledged. Callers should not read `body` after the upload
54+
* completes.
55+
*/
56+
body: Buffer | undefined;
5257
/** Presigned URL from `requestUploadUrls`. */
5358
url: string;
5459
/** Headers the platform required us to send (incl. Content-Md5, Content-Type). */
@@ -63,11 +68,17 @@ export interface DirectR2PutItem {
6368
*
6469
* Logs `r2_put_start` / `r2_put_end` per asset, plus `r2_put_retry` on
6570
* each retry attempt. Per-asset durations help identify slow uploads.
71+
*
72+
* Memory: each successful PUT clears `item.body` and invokes `onAfterPut`
73+
* so the caller can drop its own ref (e.g. delete from the in-memory
74+
* `assetFiles` cache). For huge deploys this keeps peak memory bounded by
75+
* the upload concurrency rather than the total bundle size.
6676
*/
6777
export async function uploadAssetsToR2(
6878
items: Array<DirectR2PutItem>,
6979
cliLog: CliLogger | undefined,
7080
fetchImpl: typeof fetch = fetch,
81+
onAfterPut?: (item: DirectR2PutItem) => void,
7182
): Promise<void> {
7283
if (items.length === 0) {
7384
return;
@@ -97,6 +108,12 @@ export async function uploadAssetsToR2(
97108
}
98109
return;
99110
}
111+
// Drop refs to the asset bytes now that R2 has them. Both the
112+
// local `body` ref on the item and any caller-side ref (via
113+
// `onAfterPut`) are released so the buffer becomes eligible
114+
// for GC under bounded concurrency.
115+
items[i].body = undefined;
116+
onAfterPut?.(items[i]);
100117
}
101118
})(),
102119
);
@@ -112,11 +129,18 @@ async function uploadOne(
112129
cliLog: CliLogger | undefined,
113130
fetchImpl: typeof fetch,
114131
): Promise<void> {
132+
// Capture the body up-front. The pool only clears `item.body` AFTER
133+
// `uploadOne` returns successfully, so it is guaranteed defined here for
134+
// all attempts of this single PUT.
135+
if (!item.body) {
136+
throw new Error(`internal: uploadOne invoked for ${item.path} with no body`);
137+
}
138+
const body = item.body;
115139
const startedAt = Date.now();
116140
cliLog?.info('r2_put_start', {
117141
hash: item.hash,
118142
path: item.path,
119-
sizeBytes: item.body.length,
143+
sizeBytes: body.length,
120144
});
121145

122146
let attempt = 0;
@@ -128,7 +152,7 @@ async function uploadOne(
128152
// Clone the buffer into a fresh Uint8Array view so each retry sees
129153
// an independent body. Reusing the same Buffer across retries is
130154
// safe with `fetch` (it consumes once), but explicit here.
131-
body: new Uint8Array(item.body) as unknown as BodyInit,
155+
body: new Uint8Array(body) as unknown as BodyInit,
132156
headers: item.headers,
133157
});
134158
} catch (err) {

‎packages/void/test/integration/direct-r2-deploy.test.ts‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,83 @@ describe('direct-to-R2 deploy flow', () => {
324324
expect(uniquePaths.size).toBe(4);
325325
});
326326

327+
it('prunes the in-memory assetFiles cache to needed-only after preflight', async () => {
328+
// 4 fixture assets, only 1 needed: pruning should drop 3 buffers
329+
// immediately after preflight, and the per-PUT cleanup should drop
330+
// the last one once R2 acks the upload. We assert via the JSONL log,
331+
// which captures both the pre/post-prune sizes (`asset_cache_pruned`)
332+
// and the upload events (`r2_put_end`).
333+
installPlatformMocks({
334+
preflightNeeded: ['/index.html'],
335+
preflightSkipped: 3,
336+
});
337+
const cliLog = openDeployLog();
338+
const logPath = cliLog.path;
339+
expect(logPath).toBeTruthy();
340+
const client = new PlatformClient('test-token', { apiUrl: 'https://api.test', cliLog });
341+
342+
await runStaticDeploy(
343+
workdir,
344+
{ projectId: 'proj_t', slug: 'test' },
345+
client,
346+
{ buildCommand: null, outputDir: assetsDir, appType: 'spa' },
347+
true,
348+
null,
349+
undefined,
350+
undefined,
351+
undefined,
352+
undefined,
353+
undefined,
354+
cliLog,
355+
false,
356+
);
357+
358+
await cliLog.close();
359+
const lines = readFileSync(logPath as string, 'utf-8')
360+
.trim()
361+
.split('\n')
362+
.map((l) => JSON.parse(l) as { msg: string; before?: number; after?: number });
363+
const prune = lines.find((l) => l.msg === 'asset_cache_pruned');
364+
expect(prune).toBeDefined();
365+
// Before prune: 4 fixture buffers in the cache. After: 1 (the only
366+
// needed asset). Per-PUT cleanup runs after this event so the log
367+
// value here is the post-prune-but-pre-PUT size.
368+
expect(prune?.before).toBe(4);
369+
expect(prune?.after).toBe(1);
370+
});
371+
372+
it('does not log asset_cache_pruned when nothing was skipped', async () => {
373+
// All 4 needed → prune is a no-op and we suppress the noisy log.
374+
installPlatformMocks();
375+
const cliLog = openDeployLog();
376+
const logPath = cliLog.path;
377+
expect(logPath).toBeTruthy();
378+
const client = new PlatformClient('test-token', { apiUrl: 'https://api.test', cliLog });
379+
380+
await runStaticDeploy(
381+
workdir,
382+
{ projectId: 'proj_t', slug: 'test' },
383+
client,
384+
{ buildCommand: null, outputDir: assetsDir, appType: 'spa' },
385+
true,
386+
null,
387+
undefined,
388+
undefined,
389+
undefined,
390+
undefined,
391+
undefined,
392+
cliLog,
393+
false,
394+
);
395+
396+
await cliLog.close();
397+
const lines = readFileSync(logPath as string, 'utf-8')
398+
.trim()
399+
.split('\n')
400+
.map((l) => JSON.parse(l) as { msg: string });
401+
expect(lines.find((l) => l.msg === 'asset_cache_pruned')).toBeUndefined();
402+
});
403+
327404
it('skips upload-urls + R2 PUT phase when no assets are needed', async () => {
328405
const state = installPlatformMocks({ preflightNeeded: [], preflightSkipped: 4 });
329406
const client = makeClient();

‎packages/void/test/unit/direct-r2-upload.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,37 @@ describe('uploadAssetsToR2', () => {
160160
});
161161
expect(fetchMock).toHaveBeenCalledTimes(4);
162162
});
163+
164+
it('clears item.body and invokes onAfterPut after each successful PUT', async () => {
165+
const fetchMock = vi.fn().mockResolvedValue(new Response(null, { status: 200 }));
166+
const items = [makeItem('/a.html', 'h1'), makeItem('/b.css', 'h2'), makeItem('/c.js', 'h3')];
167+
const seen: Array<string> = [];
168+
169+
await uploadAssetsToR2(items, cliLog, fetchMock, (item) => {
170+
// body must already be cleared by the time the callback runs so the
171+
// caller does not have to inspect it.
172+
expect(item.body).toBeUndefined();
173+
seen.push(item.path);
174+
});
175+
176+
// Every item invoked the callback exactly once, with body cleared.
177+
expect(seen.sort()).toEqual(['/a.html', '/b.css', '/c.js']);
178+
for (const item of items) {
179+
expect(item.body).toBeUndefined();
180+
}
181+
});
182+
183+
it('does not invoke onAfterPut when the PUT fails', async () => {
184+
const fetchMock = vi.fn().mockResolvedValue(new Response('SignatureMismatch', { status: 403 }));
185+
const item = makeItem('/oops.html', 'h-oops');
186+
const onAfterPut = vi.fn();
187+
188+
await expect(uploadAssetsToR2([item], cliLog, fetchMock, onAfterPut)).rejects.toBeDefined();
189+
190+
// Failed upload must not signal "done" via the cleanup hook — callers
191+
// depend on this to avoid prematurely dropping refs they may need to
192+
// re-presign / retry against in a future iteration.
193+
expect(onAfterPut).not.toHaveBeenCalled();
194+
expect(item.body).toBeDefined();
195+
});
163196
});

0 commit comments

Comments
 (0)