Skip to content

Commit 69b10e3

Browse files
committed
fix(file-search): stop cleanup starting a batch it cannot fund
The batch loop admitted another batch whenever any budget remained, then passed that remainder through as the batch's statement timeout, clamped to 1ms. A batch admitted with a sliver left either runs past the budget it was given or aborts on its own statement timeout, which the caller reports as a cleanup failure rather than as work still to do. Stop once less than one batch's nominal share of the budget remains. The floor is derived from the budget and the batch cap rather than fixed, so retuning either cannot leave it admitting batches at more than their share again.
1 parent bad0ce4 commit 69b10e3

3 files changed

Lines changed: 38 additions & 4 deletions

File tree

‎apps/sim/lib/workspace-files/search/chunks.integration.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ vi.mock('@/lib/file-parsers', () => ({ parseBuffer: vi.fn(), isSupportedFileType
2525

2626
import {
2727
FILE_SEARCH_CLEANUP_BATCH_ROWS,
28+
FILE_SEARCH_CLEANUP_BUDGET_MS,
2829
FILE_SEARCH_CLEANUP_MAX_BATCHES,
2930
} from '@/lib/workspace-files/search/constants'
3031
import { prepareWorkspaceFileSearchDispatch } from '@/lib/workspace-files/search/dispatcher'
@@ -373,6 +374,28 @@ describe('chunked workspace file search on PostgreSQL', () => {
373374
(await connection`SELECT count(*)::int AS count FROM workspace_file_search_chunk`)[0].count
374375
).toBe(0)
375376
})
377+
it('ends a cleanup run cleanly when its time budget runs out', async () => {
378+
const build = (await beginFileSearchBuild(revision))!
379+
await connection`INSERT INTO workspace_file_search_chunk (build_id, workspace_id, ordinal, line_start, fragment, content)
380+
SELECT ${build.id}, 'workspace-1', n, n + 1, false, 'x' FROM generate_series(0, 999) n`
381+
await connection`UPDATE workspace_file_search_build SET expires_at = now() WHERE id = ${build.id}`
382+
383+
/** The deadline is read first; every read after it reports a budget all but consumed. */
384+
const startedAt = Date.now()
385+
const clock = vi
386+
.spyOn(Date, 'now')
387+
.mockReturnValueOnce(startedAt)
388+
.mockReturnValue(startedAt + FILE_SEARCH_CLEANUP_BUDGET_MS - 1)
389+
try {
390+
await expect(cleanupFileSearchBuilds()).resolves.toBe(0)
391+
} finally {
392+
clock.mockRestore()
393+
}
394+
395+
expect(
396+
(await connection`SELECT count(*)::int AS count FROM workspace_file_search_chunk`)[0].count
397+
).toBe(1000)
398+
})
376399
it('retires many small builds within one cleanup run', async () => {
377400
await connection`INSERT INTO workspace_file_search_build (id, file_id, workspace_id, source_content_updated_at, expires_at)
378401
SELECT 'retired-' || n, 'file-1', 'workspace-1', now(), now() FROM generate_series(1, 100) n`

‎apps/sim/lib/workspace-files/search/constants.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,16 @@ export const FILE_SEARCH_CLEANUP_BATCH_BUILDS = 100
3232
export const FILE_SEARCH_CLEANUP_BACKLOG_ROWS = 10000
3333
export const FILE_SEARCH_CLEANUP_MAX_BATCHES = 10
3434
export const FILE_SEARCH_CLEANUP_BUDGET_MS = 5000
35+
/**
36+
* One batch's nominal share of the run budget, and so the smallest slice worth starting another
37+
* with.
38+
*
39+
* Running out of budget is how cleanup normally ends. A batch admitted with less than its share
40+
* either runs past the budget it was given or aborts on its own statement timeout, which the caller
41+
* reports as a cleanup failure rather than as work still to do.
42+
*/
43+
export const FILE_SEARCH_CLEANUP_MIN_BATCH_MS =
44+
FILE_SEARCH_CLEANUP_BUDGET_MS / FILE_SEARCH_CLEANUP_MAX_BATCHES
3545
export const FILE_SEARCH_RECONCILE_INTERVAL_MS = 60 * 60 * 1000
3646
export const FILE_SEARCH_INSERT_BATCH_ROWS = 250
3747
export const FILE_SEARCH_INSERT_BATCH_BYTES = 1024 * 1024

‎apps/sim/lib/workspace-files/search/index-state.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
FILE_SEARCH_CLEANUP_BATCH_ROWS,
1515
FILE_SEARCH_CLEANUP_BUDGET_MS,
1616
FILE_SEARCH_CLEANUP_MAX_BATCHES,
17+
FILE_SEARCH_CLEANUP_MIN_BATCH_MS,
1718
FILE_SEARCH_INSERT_BATCH_BYTES,
1819
FILE_SEARCH_INSERT_BATCH_ROWS,
1920
} from '@/lib/workspace-files/search/constants'
@@ -257,11 +258,11 @@ export async function failFileSearchRevision(
257258
export async function cleanupFileSearchBuilds(): Promise<number> {
258259
const deadline = Date.now() + FILE_SEARCH_CLEANUP_BUDGET_MS
259260
let deleted = 0
260-
for (let batch = 0; batch < FILE_SEARCH_CLEANUP_MAX_BATCHES && Date.now() < deadline; batch++) {
261+
for (let batch = 0; batch < FILE_SEARCH_CLEANUP_MAX_BATCHES; batch++) {
262+
const remainingBudget = deadline - Date.now()
263+
if (remainingBudget < FILE_SEARCH_CLEANUP_MIN_BATCH_MS) break
261264
const result = await db.transaction(async (tx) => {
262-
await configureFileSearchTransaction(tx, {
263-
statementTimeout: Math.max(1, deadline - Date.now()),
264-
})
265+
await configureFileSearchTransaction(tx, { statementTimeout: remainingBudget })
265266
const builds = await tx.execute<{
266267
id: string
267268
}>(sql`SELECT id FROM workspace_file_search_build

0 commit comments

Comments
 (0)