Skip to content

Commit 9469c10

Browse files
committed
fix(tables): make TTL cleanup fair across tables
1 parent 036d0f6 commit 9469c10

2 files changed

Lines changed: 75 additions & 16 deletions

File tree

apps/sim/background/cleanup-table-row-ttl.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,45 @@ describe('table row TTL cleanup', () => {
125125
expect(mockSignalTableRowsChanged).toHaveBeenCalledTimes(1)
126126
})
127127

128+
it('gives each table one batch before returning to a backlogged table', async () => {
129+
const secondTable = {
130+
...table,
131+
id: 'table-2',
132+
}
133+
const attemptedTableIds: string[] = []
134+
const tableAttempts = new Map<string, number>()
135+
mockListExecute.mockResolvedValue([
136+
{ id: table.id, workspaceId: table.workspaceId },
137+
{ id: secondTable.id, workspaceId: secondTable.workspaceId },
138+
])
139+
mockWithLockedTable.mockImplementation(async (tableId, mutate) => {
140+
const freshTable = tableId === secondTable.id ? secondTable : table
141+
return mutate(freshTable, {
142+
execute: vi.fn(async () => {
143+
attemptedTableIds.push(tableId)
144+
const attempt = (tableAttempts.get(tableId) ?? 0) + 1
145+
tableAttempts.set(tableId, attempt)
146+
if (tableId === table.id && attempt === 1) {
147+
return [{ count: 500, lastId: 'row-500' }]
148+
}
149+
if (tableId === secondTable.id) {
150+
return [{ count: 1, lastId: 'row-1' }]
151+
}
152+
return [{ count: 0, lastId: null }]
153+
}),
154+
})
155+
})
156+
157+
await expect(runCleanupTableRowTtl()).resolves.toEqual({
158+
batches: 3,
159+
deleted: 501,
160+
limitReached: false,
161+
})
162+
expect(attemptedTableIds).toEqual([table.id, secondTable.id, table.id])
163+
expect(mockSignalTableRowsChanged).toHaveBeenCalledWith(table.id)
164+
expect(mockSignalTableRowsChanged).toHaveBeenCalledWith(secondTable.id)
165+
})
166+
128167
it('registers one serialized Trigger.dev task', () => {
129168
expect(cleanupTableRowTtlTask).toEqual(
130169
expect.objectContaining({

apps/sim/background/cleanup-table-row-ttl.ts

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ interface DeletedTtlBatch {
2828
lastId: string | null
2929
}
3030

31+
interface TtlTableCleanupState {
32+
ref: ExpiredTtlTableRef
33+
afterId?: string
34+
deleted: number
35+
complete: boolean
36+
}
37+
3138
export interface TableRowTtlCleanupResult {
3239
batches: number
3340
deleted: number
@@ -64,7 +71,9 @@ async function listExpiredTtlTables(nowEpochSeconds: number): Promise<ExpiredTtl
6471
)
6572
)::numeric <= ${nowEpochSeconds}
6673
)
67-
ORDER BY ${userTableDefinitions.id}
74+
ORDER BY
75+
md5(${userTableDefinitions.id} || ${nowEpochSeconds}::text),
76+
${userTableDefinitions.id}
6877
LIMIT ${TTL_CLEANUP_MAX_BATCHES}
6978
`)
7079
return Array.isArray(rows) ? rows : []
@@ -169,33 +178,44 @@ export async function runCleanupTableRowTtl(
169178

170179
const nowEpochSeconds = Math.floor(Date.now() / 1000)
171180
const tableRefs = await listExpiredTtlTables(nowEpochSeconds)
181+
const tableStates: TtlTableCleanupState[] = tableRefs.map((ref) => ({
182+
ref,
183+
deleted: 0,
184+
complete: false,
185+
}))
172186
let deleted = 0
173187
let batches = 0
174-
let lastBatchDeleted = 0
175-
176-
for (const ref of tableRefs) {
177-
let afterId: string | undefined
178-
let tableDeleted = 0
179188

180-
while (batches < TTL_CLEANUP_MAX_BATCHES && !signal?.aborted) {
181-
const batch = await deleteExpiredRowsForTable(ref, nowEpochSeconds, afterId)
182-
if (!batch.attempted) break
189+
while (
190+
batches < TTL_CLEANUP_MAX_BATCHES &&
191+
!signal?.aborted &&
192+
tableStates.some((state) => !state.complete)
193+
) {
194+
for (const state of tableStates) {
195+
if (state.complete) continue
196+
if (batches === TTL_CLEANUP_MAX_BATCHES || signal?.aborted) break
197+
198+
const batch = await deleteExpiredRowsForTable(state.ref, nowEpochSeconds, state.afterId)
199+
if (!batch.attempted) {
200+
state.complete = true
201+
continue
202+
}
183203

184204
batches++
185205
deleted += batch.deleted
186-
tableDeleted += batch.deleted
187-
lastBatchDeleted = batch.deleted
188-
afterId = batch.lastId ?? undefined
189-
if (batch.deleted < TTL_CLEANUP_BATCH_SIZE) break
206+
state.deleted += batch.deleted
207+
state.afterId = batch.lastId ?? undefined
208+
if (batch.deleted < TTL_CLEANUP_BATCH_SIZE) state.complete = true
190209
}
210+
}
191211

192-
if (tableDeleted > 0) signalTableRowsChanged(ref.id)
193-
if (batches === TTL_CLEANUP_MAX_BATCHES || signal?.aborted) break
212+
for (const state of tableStates) {
213+
if (state.deleted > 0) signalTableRowsChanged(state.ref.id)
194214
}
195215

196216
const limitReached =
197217
batches === TTL_CLEANUP_MAX_BATCHES &&
198-
(lastBatchDeleted === TTL_CLEANUP_BATCH_SIZE || tableRefs.length === TTL_CLEANUP_MAX_BATCHES)
218+
(tableStates.some((state) => !state.complete) || tableRefs.length === TTL_CLEANUP_MAX_BATCHES)
199219
logger.info('Table row TTL cleanup completed', { batches, deleted, limitReached })
200220
return { batches, deleted, limitReached }
201221
}

0 commit comments

Comments
 (0)