Skip to content

Commit 5a00005

Browse files
committed
fix(file-search): measure the cleanup budget after the connection is held
Hoisting the remaining-budget read out of the transaction callback made it describe the moment the batch was admitted rather than the moment its statements begin. Time spent waiting for a pooled connection then went unaccounted, and the batch installed a timeout larger than the budget actually left. Keep the cheap check before opening a transaction, and re-read once the connection is in hand so the installed timeout is the budget that remains.
1 parent 69b10e3 commit 5a00005

2 files changed

Lines changed: 27 additions & 2 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
@@ -396,6 +396,29 @@ describe('chunked workspace file search on PostgreSQL', () => {
396396
(await connection`SELECT count(*)::int AS count FROM workspace_file_search_chunk`)[0].count
397397
).toBe(1000)
398398
})
399+
it('abandons a batch whose budget was spent acquiring its connection', async () => {
400+
const build = (await beginFileSearchBuild(revision))!
401+
await connection`INSERT INTO workspace_file_search_chunk (build_id, workspace_id, ordinal, line_start, fragment, content)
402+
SELECT ${build.id}, 'workspace-1', n, n + 1, false, 'x' FROM generate_series(0, 999) n`
403+
await connection`UPDATE workspace_file_search_build SET expires_at = now() WHERE id = ${build.id}`
404+
405+
/** Full budget when the batch is admitted, none left once its connection is in hand. */
406+
const startedAt = Date.now()
407+
const clock = vi
408+
.spyOn(Date, 'now')
409+
.mockReturnValueOnce(startedAt)
410+
.mockReturnValueOnce(startedAt)
411+
.mockReturnValue(startedAt + FILE_SEARCH_CLEANUP_BUDGET_MS - 1)
412+
try {
413+
await expect(cleanupFileSearchBuilds()).resolves.toBe(0)
414+
} finally {
415+
clock.mockRestore()
416+
}
417+
418+
expect(
419+
(await connection`SELECT count(*)::int AS count FROM workspace_file_search_chunk`)[0].count
420+
).toBe(1000)
421+
})
399422
it('retires many small builds within one cleanup run', async () => {
400423
await connection`INSERT INTO workspace_file_search_build (id, file_id, workspace_id, source_content_updated_at, expires_at)
401424
SELECT 'retired-' || n, 'file-1', 'workspace-1', now(), now() FROM generate_series(1, 100) n`

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,9 +259,11 @@ export async function cleanupFileSearchBuilds(): Promise<number> {
259259
const deadline = Date.now() + FILE_SEARCH_CLEANUP_BUDGET_MS
260260
let deleted = 0
261261
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
262+
if (deadline - Date.now() < FILE_SEARCH_CLEANUP_MIN_BATCH_MS) break
264263
const result = await db.transaction(async (tx) => {
264+
/** Re-read: acquiring the connection can itself have spent the rest of the budget. */
265+
const remainingBudget = deadline - Date.now()
266+
if (remainingBudget < FILE_SEARCH_CLEANUP_MIN_BATCH_MS) return null
265267
await configureFileSearchTransaction(tx, { statementTimeout: remainingBudget })
266268
const builds = await tx.execute<{
267269
id: string

0 commit comments

Comments
 (0)