Skip to content

Commit 8fea7cf

Browse files
feat(cleanup): add per-type row limits to existing jobs (#7842)
* feat(cleanup): add bounded manual retention runs * fix(cleanup): coordinate deletion with resource eligibility * fix(cleanup): persist storage retries and complete test fixtures * fix(cleanup): guard storage retries by file generation * improvement(cleanup): reuse existing jobs for row limits * fix(cleanup): fail manual jobs on owner lookup errors
1 parent bb20239 commit 8fea7cf

18 files changed

Lines changed: 780 additions & 123 deletions

File tree

apps/sim/app/api/cron/cleanup-soft-deletes/route.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,36 @@
11
import { createLogger } from '@sim/logger'
22
import { type NextRequest, NextResponse } from 'next/server'
3+
import { softDeletesCleanupContract } from '@/lib/api/contracts/cleanup'
4+
import { parseRequest } from '@/lib/api/server/validation'
35
import { verifyCronAuth } from '@/lib/auth/internal'
4-
import { dispatchCleanupJobs } from '@/lib/billing/cleanup-dispatcher'
6+
import { dispatchBoundedCleanup, dispatchCleanupJobs } from '@/lib/billing/cleanup-dispatcher'
57
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
68

79
export const dynamic = 'force-dynamic'
810

911
const logger = createLogger('SoftDeleteCleanupAPI')
1012

13+
/** Cron-secret maintenance protocol is global; workspace principal authorization does not apply. */
1114
export const GET = withRouteHandler(async (request: NextRequest) => {
1215
try {
1316
const authError = verifyCronAuth(request, 'soft-delete cleanup')
1417
if (authError) return authError
1518

19+
const parsed = await parseRequest(
20+
softDeletesCleanupContract,
21+
request,
22+
{},
23+
{
24+
rejectDuplicateQueryValues: true,
25+
rejectBlankQueryValues: true,
26+
}
27+
)
28+
if (!parsed.success) return parsed.response
29+
if (parsed.data.query) {
30+
const result = await dispatchBoundedCleanup('cleanup-soft-deletes', parsed.data.query)
31+
return NextResponse.json(result, { status: 202 })
32+
}
33+
1634
const result = await dispatchCleanupJobs('cleanup-soft-deletes')
1735

1836
logger.info('Soft-delete cleanup jobs dispatched', result)
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { createMockRequest } from '@sim/testing'
2+
import { beforeEach, describe, expect, it, vi } from 'vitest'
3+
4+
const { auth, bounded, scheduled } = vi.hoisted(() => ({
5+
auth: vi.fn(),
6+
bounded: vi.fn(),
7+
scheduled: vi.fn(),
8+
}))
9+
vi.mock('@/lib/auth/internal', () => ({ verifyCronAuth: auth }))
10+
vi.mock('@/lib/billing/cleanup-dispatcher', () => ({
11+
dispatchBoundedCleanup: bounded,
12+
dispatchCleanupJobs: scheduled,
13+
}))
14+
15+
import { GET as softDeletes } from '@/app/api/cron/cleanup-soft-deletes/route'
16+
import { GET as logs } from '@/app/api/logs/cleanup/route'
17+
18+
for (const [path, GET, type, limit] of [
19+
['/api/logs/cleanup', logs, 'cleanup-logs', 'workflowLogs'],
20+
['/api/cron/cleanup-soft-deletes', softDeletes, 'cleanup-soft-deletes', 'workflows'],
21+
] as const) {
22+
describe(path, () => {
23+
beforeEach(() => {
24+
vi.clearAllMocks()
25+
auth.mockReturnValue(null)
26+
bounded.mockResolvedValue({ triggered: true, runId: 'run-one', limits: { [limit]: 2 } })
27+
scheduled.mockResolvedValue({
28+
jobIds: ['batch-one'],
29+
jobCount: 1,
30+
chunkCount: 2,
31+
workspaceCount: 3,
32+
})
33+
})
34+
const request = (query = '') =>
35+
createMockRequest('GET', undefined, {}, `http://localhost:3000${path}${query}`)
36+
it('authenticates before parsing invalid limits', async () => {
37+
auth.mockReturnValue(new Response(null, { status: 401 }))
38+
expect((await GET(request('?unknown=1'))).status).toBe(401)
39+
expect(bounded).not.toHaveBeenCalled()
40+
expect(scheduled).not.toHaveBeenCalled()
41+
})
42+
it('keeps no-parameter scheduled dispatch unchanged', async () => {
43+
const response = await GET(request())
44+
expect(response.status).toBe(200)
45+
expect(scheduled).toHaveBeenCalledWith(type)
46+
expect(bounded).not.toHaveBeenCalled()
47+
})
48+
it('accepts one bounded run', async () => {
49+
const response = await GET(request(`?${limit}=2`))
50+
expect(response.status).toBe(202)
51+
expect(bounded).toHaveBeenCalledWith(type, { [limit]: 2 })
52+
expect(await response.json()).toEqual({
53+
triggered: true,
54+
runId: 'run-one',
55+
limits: { [limit]: 2 },
56+
})
57+
expect(scheduled).not.toHaveBeenCalled()
58+
})
59+
it.each(['?dryRun=true', '?unknown=1', '?batchSize=3', `?${limit}=2&${limit}=3`])(
60+
'rejects invalid query %s',
61+
async (query) => {
62+
expect((await GET(request(query))).status).toBe(400)
63+
expect(bounded).not.toHaveBeenCalled()
64+
expect(scheduled).not.toHaveBeenCalled()
65+
}
66+
)
67+
it('reports a dispatch failure', async () => {
68+
bounded.mockRejectedValue(new Error('Trigger unavailable'))
69+
expect((await GET(request(`?${limit}=2`))).status).toBe(500)
70+
})
71+
})
72+
}

apps/sim/app/api/logs/cleanup/route.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,36 @@
11
import { createLogger } from '@sim/logger'
22
import { type NextRequest, NextResponse } from 'next/server'
3+
import { logsCleanupContract } from '@/lib/api/contracts/cleanup'
4+
import { parseRequest } from '@/lib/api/server/validation'
35
import { verifyCronAuth } from '@/lib/auth/internal'
4-
import { dispatchCleanupJobs } from '@/lib/billing/cleanup-dispatcher'
6+
import { dispatchBoundedCleanup, dispatchCleanupJobs } from '@/lib/billing/cleanup-dispatcher'
57
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
68

79
export const dynamic = 'force-dynamic'
810

911
const logger = createLogger('LogsCleanupAPI')
1012

13+
/** Cron-secret maintenance protocol is global; workspace principal authorization does not apply. */
1114
export const GET = withRouteHandler(async (request: NextRequest) => {
1215
try {
1316
const authError = verifyCronAuth(request, 'logs cleanup')
1417
if (authError) return authError
1518

19+
const parsed = await parseRequest(
20+
logsCleanupContract,
21+
request,
22+
{},
23+
{
24+
rejectDuplicateQueryValues: true,
25+
rejectBlankQueryValues: true,
26+
}
27+
)
28+
if (!parsed.success) return parsed.response
29+
if (parsed.data.query) {
30+
const result = await dispatchBoundedCleanup('cleanup-logs', parsed.data.query)
31+
return NextResponse.json(result, { status: 202 })
32+
}
33+
1634
const result = await dispatchCleanupJobs('cleanup-logs')
1735

1836
logger.info('Log cleanup jobs dispatched', result)

apps/sim/background/cleanup-logs.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,12 @@ const {
4545
mockTask: vi.fn((config: unknown) => config),
4646
}))
4747

48-
vi.mock('@trigger.dev/sdk', () => ({ task: mockTask }))
48+
vi.mock('@trigger.dev/sdk', () => ({ task: mockTask, queue: vi.fn((config) => config) }))
49+
50+
vi.mock('@/lib/billing/cleanup-dispatcher', () => ({ runCleanupWithLimits: vi.fn() }))
4951

5052
vi.mock('@/lib/cleanup/batch-delete', () => ({
53+
consumeRowBudget: vi.fn(),
5154
batchDeleteByWorkspaceAndTimestamp: mockBatchDeleteByWorkspaceAndTimestamp,
5255
chunkedBatchDelete: mockChunkedBatchDelete,
5356
}))
@@ -199,7 +202,7 @@ describe('cleanup logs worker', () => {
199202

200203
it('caps Trigger.dev concurrency for log cleanup tasks', () => {
201204
expect(cleanupLogsTask).toMatchObject({
202-
queue: { concurrencyLimit: 2 },
205+
queue: { name: 'retention-cleanup', concurrencyLimit: 1 },
203206
})
204207
})
205208
})

0 commit comments

Comments
 (0)