Skip to content

Commit 2cba4fa

Browse files
committed
fix(projects): finish a retired Project's storage sweep only on the provider's last page
Azure Blob, GCS and S3-compatible providers can return a short or empty page that is not the last, which the sweep read as done and left the rest of the prefix orphaned. The sweep now ends only on page.done, and follows a non-final page that removed nothing to the next one within the run.
1 parent 734287e commit 2cba4fa

3 files changed

Lines changed: 75 additions & 15 deletions

File tree

‎apps/sim/lib/projects/files/prefix-cleanup.test.ts‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,15 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
1010
vi.mock('@/lib/uploads/core/setup.server', () => uploadsSetupMock)
1111
vi.mock('@/lib/uploads/config', () => uploadsConfigMock)
1212
vi.mock('@/lib/uploads/server/metadata', () => uploadsMetadataMock)
13+
vi.mock('@/lib/uploads/core/prefix-sweep', async (importOriginal) => {
14+
const actual = await importOriginal<typeof import('@/lib/uploads/core/prefix-sweep')>()
15+
return { ...actual, listStorageKeyPage: vi.fn(actual.listStorageKeyPage) }
16+
})
1317

1418
import type { OutboxEventContext } from '@/lib/core/outbox/service'
1519
import { PROJECT_FILE_PREFIX_CLEANUP_EVENT } from '@/lib/projects/files/outbox-events'
1620
import { projectFilePrefixCleanupOutboxHandlers } from '@/lib/projects/files/prefix-cleanup'
21+
import { listStorageKeyPage } from '@/lib/uploads/core/prefix-sweep'
1722

1823
const cleanup = projectFilePrefixCleanupOutboxHandlers[PROJECT_FILE_PREFIX_CLEANUP_EVENT]
1924

@@ -68,12 +73,56 @@ describe('Project prefix cleanup on local storage', () => {
6873
expect(await exists('project/p10/keep.txt')).toBe(true)
6974
})
7075

71-
it('spends an attempt when a full page holds only objects it cannot remove', async () => {
76+
it('reaches objects behind a full page it cannot remove', async () => {
7277
for (let index = 0; index < 520; index++) await put(`project/p1/legacy file ${index}.txt`)
78+
await put('project/p1/zz.txt')
79+
80+
await expect(
81+
cleanup({ projectId: 'p1' }, outboxContext(Date.now() + 60_000))
82+
).resolves.toBeUndefined()
83+
84+
expect(await exists('project/p1/zz.txt')).toBe(false)
85+
})
86+
87+
it('keeps sweeping after a short page that is not the last', async () => {
88+
await put('project/p1/a.txt')
89+
await put('project/p1/b.txt')
90+
vi.mocked(listStorageKeyPage).mockResolvedValueOnce({
91+
keys: ['project/p1/a.txt'],
92+
done: false,
93+
nextPageToken: 'next',
94+
})
95+
96+
const result = await cleanup({ projectId: 'p1' }, outboxContext())
97+
98+
expect(result).toMatchObject({ outcome: 'deferred', consumeAttempt: false })
99+
expect(await exists('project/p1/a.txt')).toBe(false)
100+
expect(await exists('project/p1/b.txt')).toBe(true)
101+
})
102+
103+
it('follows an empty page that is not the last to the objects after it', async () => {
104+
await put('project/p1/a.txt')
105+
vi.mocked(listStorageKeyPage)
106+
.mockResolvedValueOnce({ keys: [], done: false, nextPageToken: 'next' })
107+
.mockResolvedValueOnce({ keys: ['project/p1/a.txt'], done: true })
108+
109+
await expect(cleanup({ projectId: 'p1' }, outboxContext())).resolves.toBeUndefined()
110+
111+
expect(vi.mocked(listStorageKeyPage).mock.calls[1]?.[3]).toBe('next')
112+
expect(await exists('project/p1/a.txt')).toBe(false)
113+
})
114+
115+
it('spends an attempt when its listing stops advancing without removing anything', async () => {
116+
vi.mocked(listStorageKeyPage).mockResolvedValue({
117+
keys: [],
118+
done: false,
119+
nextPageToken: 'stuck',
120+
})
73121

74-
const result = await cleanup({ projectId: 'p1' }, outboxContext(Date.now() + 60_000))
122+
const result = await cleanup({ projectId: 'p1' }, outboxContext())
75123

76124
expect(result).toMatchObject({ outcome: 'deferred' })
77125
expect(result).not.toMatchObject({ consumeAttempt: false })
126+
expect(listStorageKeyPage).toHaveBeenCalledTimes(2)
78127
})
79128
})

‎apps/sim/lib/projects/files/prefix-cleanup.ts‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import {
2121
deleteStorageKeysUnderPrefix,
2222
listStorageKeyPage,
2323
outboxDeadlineNear,
24-
STORAGE_KEY_PAGE_SIZE,
2524
storageCleanupAvailableAt,
2625
} from '@/lib/uploads/core/prefix-sweep'
2726
import { abortProviderUpload } from '@/lib/uploads/upload-session/provider'
@@ -202,21 +201,33 @@ export async function queueRetiredProjectUploadCleanup(
202201
})
203202
}
204203

205-
/** `stalled` when a page that must be re-listed removed nothing, so continuing cannot progress. */
204+
/**
205+
* Deletes one listing page's objects per run, re-listing from the prefix start next time. A
206+
* provider (Azure Blob, GCS, S3-compatibles) may return a short or empty page that is not the last,
207+
* so only `page.done` ends the sweep; a non-final page that removes nothing is followed to the next
208+
* one within the run, so objects behind it are still reached. `stalled` when no page removed
209+
* anything before the listing stopped advancing or the deadline neared.
210+
*/
206211
async function sweepPrefix(
207212
projectId: string,
208213
context: OutboxEventContext
209214
): Promise<'done' | 'continue' | 'stalled'> {
210215
const prefix = prefixFor(projectId)
211-
const page = await listStorageKeyPage(prefix, 'project', context.signal)
212-
const { deleted, complete } = await deleteStorageKeysUnderPrefix(
213-
prefix,
214-
'project',
215-
page.keys,
216-
context
217-
)
218-
if (complete && page.keys.length < STORAGE_KEY_PAGE_SIZE) return 'done'
219-
return deleted > 0 ? 'continue' : 'stalled'
216+
let pageToken: string | undefined
217+
for (;;) {
218+
const page = await listStorageKeyPage(prefix, 'project', context.signal, pageToken)
219+
const { deleted, complete } = await deleteStorageKeysUnderPrefix(
220+
prefix,
221+
'project',
222+
page.keys,
223+
context
224+
)
225+
if (complete && page.done) return 'done'
226+
if (deleted > 0) return 'continue'
227+
const advanced = page.nextPageToken !== undefined && page.nextPageToken !== pageToken
228+
if (!complete || !advanced || outboxDeadlineNear(context)) return 'stalled'
229+
pageToken = page.nextPageToken
230+
}
220231
}
221232

222233
const cleanupPrefix: OutboxHandler<unknown> = async (raw, context) => {
@@ -259,7 +270,7 @@ const cleanupPrefix: OutboxHandler<unknown> = async (raw, context) => {
259270
const sweep = await sweepPrefix(payload.projectId, context)
260271
if (sweep === 'continue') return continueOutboxHandler('Continuing Project storage cleanup')
261272
if (sweep === 'stalled')
262-
return deferOutboxHandler('Project storage cleanup removed nothing from a full listing page')
273+
return deferOutboxHandler('Project storage cleanup removed nothing from its listing')
263274
}
264275

265276
/**

‎apps/sim/lib/uploads/core/prefix-sweep.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { deleteFile } from '@/lib/uploads/core/storage-service'
1616
import { UPLOAD_URL_TTL_MS } from '@/lib/uploads/upload-session/provider'
1717

1818
/** Most keys one {@link listStorageKeyPage} call returns. */
19-
export const STORAGE_KEY_PAGE_SIZE = 500
19+
const STORAGE_KEY_PAGE_SIZE = 500
2020
const CLOCK_SKEW_MS = 60_000
2121
const DEADLINE_MARGIN_MS = 5000
2222

0 commit comments

Comments
 (0)