Skip to content

Commit 67d1976

Browse files
committed
fix(search): lock each purge batch's rows before deleting them by tuple id
A concurrent update could move every chosen row to a new tuple id before the delete, so the batch deleted nothing and the purge reported the knowledge base finished with rows left. The batch now locks its rows in one statement and deletes them in the next, whose snapshot sees the current versions
1 parent ae18bda commit 67d1976

2 files changed

Lines changed: 37 additions & 6 deletions

File tree

‎packages/db/scripts/purge-search-projection.integration.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
SearchProjectionPurgeError,
99
} from '@sim/db/scripts/purge-search-projection'
1010
import { readTestDatabaseUrl } from '@sim/db/testing/test-infrastructure'
11+
import { sleep } from '@sim/utils/helpers'
1112
import { generateId } from '@sim/utils/id'
1213
import postgres, { type Sql } from 'postgres'
1314
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'
@@ -306,6 +307,31 @@ describe('Search candidate retirement in PostgreSQL', () => {
306307
expect(await candidateCount('search')).toBe(3)
307308
})
308309

310+
it('deletes rows a concurrent update moved after the batch chose them', async () => {
311+
await seedBase({ id: 'search', width: 1536, model: 'text-embedding-3-small', chunks: 3 }, true)
312+
const writer = postgres(fixtureUrl.toString(), { max: 1, onnotice: () => undefined })
313+
try {
314+
let purge: ReturnType<typeof purgeSearchProjection> | undefined
315+
await writer.begin(async (tx) => {
316+
// Every candidate row gets a new tuple version, committed only once the purge waits on it.
317+
await tx`UPDATE embedding_search SET enabled = enabled WHERE knowledge_base_id = 'search'`
318+
purge = purgeSearchProjection(sql, { batchSize: 10, pauseMs: 0 })
319+
for (;;) {
320+
const [{ waiting }] = await tx<{ waiting: boolean }[]>`
321+
SELECT EXISTS (SELECT 1 FROM pg_stat_activity
322+
WHERE datname = current_database() AND wait_event_type = 'Lock'
323+
AND pid <> pg_backend_pid()) AS waiting`
324+
if (waiting) break
325+
await sleep(20)
326+
}
327+
})
328+
expect(await purge).toEqual({ deleted: 3, batches: 1, remaining: false })
329+
} finally {
330+
await writer.end()
331+
}
332+
expect(await candidateCount('search')).toBe(0)
333+
})
334+
309335
it('stops when its connection is replaced, since the new session lacks the purge lock', async () => {
310336
await seedBase({ id: 'search', width: 1536, model: 'text-embedding-3-small', chunks: 5 }, true)
311337
let closed: () => void = () => undefined

‎packages/db/scripts/purge-search-projection.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,10 @@ interface SearchProjectionPurgeResult {
202202

203203
/**
204204
* Deletes one batch of a Search knowledge base's candidate rows. The rows are chosen through
205-
* `embedding_search_kb_idx` and deleted by tuple id, re-checking the knowledge base so a row
206-
* moved or reassigned since the scan is left alone. Deletes nothing when the knowledge base is gone.
205+
* `embedding_search_kb_idx` and locked by one statement, then deleted by tuple id in the next.
206+
* The lock returns a concurrently updated row at its current tuple id and keeps it there, and the
207+
* delete's later snapshot sees that version, so an empty batch means no row is left. The knowledge
208+
* base re-check leaves a reassigned row alone. Deletes nothing when the knowledge base is gone.
207209
* Refuses when this backend no longer holds the purge lock: the client reconnects silently after a
208210
* dropped connection, and the new backend does not inherit the old one's session lock.
209211
*/
@@ -224,13 +226,16 @@ async function purgeBatch(sql: Sql, knowledgeBaseId: string, batchSize: number):
224226
'A knowledge base is no longer Search-marked; stopped before deleting its candidate rows'
225227
)
226228
}
229+
const [chosen] = await tx<{ ctids: string | null }[]>`
230+
SELECT array_agg(ctid)::text AS ctids FROM (
231+
SELECT ctid FROM embedding_search WHERE knowledge_base_id = ${knowledgeBaseId}
232+
LIMIT ${batchSize} FOR UPDATE
233+
) batch`
234+
if (!chosen.ctids) return 0
227235
const [row] = await tx<{ deleted: number }[]>`
228236
WITH deleted AS (
229237
DELETE FROM embedding_search
230-
WHERE ctid = ANY (ARRAY(
231-
SELECT ctid FROM embedding_search WHERE knowledge_base_id = ${knowledgeBaseId}
232-
LIMIT ${batchSize}
233-
)) AND knowledge_base_id = ${knowledgeBaseId}
238+
WHERE ctid = ANY (${chosen.ctids}::tid[]) AND knowledge_base_id = ${knowledgeBaseId}
234239
RETURNING 1
235240
)
236241
SELECT count(*)::int AS deleted FROM deleted`

0 commit comments

Comments
 (0)