Skip to content

Commit ecbda39

Browse files
committed
fix(knowledge): keep the outbox event pending until a deployment without a worker fills the projections
Without a Trigger.dev worker the outbox handler no longer detaches the backfill and completes the event; it runs one bounded slice per outbox run and yields with continueOutboxHandler until both projections are filled, so a restart loses at most one slice and the event is never marked done ahead of the work. The index builds run on one reserved connection, so the session-scoped lock timeout covers every build and its reset. Claude-Session: https://claude.ai/code/session_01XU6c7pKRpa5CMoMHKDdqxX
1 parent 8a64dd1 commit ecbda39

5 files changed

Lines changed: 113 additions & 47 deletions

File tree

‎apps/sim/lib/core/outbox/processor.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ vi.mock('@/lib/knowledge/application/slack-search/outbox', () => ({
3333
vi.mock('@/lib/knowledge/documents/processing-outbox-handler', () => ({
3434
knowledgeDocumentProcessingOutboxHandlers: {},
3535
}))
36+
vi.mock('@/lib/knowledge/search/projection-source-acl-backfill', () => ({
37+
projectionSourceAclBackfillOutboxHandlers: {},
38+
}))
3639
vi.mock('@/lib/mothership/inbox/cleanup-outbox', () => ({ inboxCleanupOutboxHandlers: {} }))
3740
vi.mock('@/lib/organizations/resource-cleanup', () => ({
3841
organizationResourceCleanupOutboxHandlers: {},

‎apps/sim/lib/knowledge/search/projection-source-acl-backfill.test.ts‎

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
*/
44
import { beforeEach, describe, expect, it, vi } from 'vitest'
55

6-
const { mockBackfill, mockEnd, mockPostgres, mockTasksTrigger } = vi.hoisted(() => ({
6+
const { mockBackfill, mockEnd, mockPostgres, mockTasksTrigger, envState } = vi.hoisted(() => ({
77
mockBackfill: vi.fn(),
88
mockEnd: vi.fn(async () => undefined),
99
mockPostgres: vi.fn(),
1010
mockTasksTrigger: vi.fn(async () => ({ id: 'run-1' })),
11+
envState: { triggerEnabled: false, secret: undefined as string | undefined },
1112
}))
1213

1314
vi.mock('@sim/db', () => ({ resolveDbUrl: () => 'postgres://localhost:5432/sim' }))
@@ -19,11 +20,27 @@ vi.mock('@sim/db/script-migrations/0021_embedding_search_connector', () => ({
1920
vi.mock('postgres', () => ({ default: mockPostgres }))
2021
vi.mock('@trigger.dev/sdk', () => ({ tasks: { trigger: mockTasksTrigger } }))
2122
vi.mock('@/lib/core/async-jobs/region', () => ({ resolveTriggerRegion: async () => 'us-east-1' }))
22-
vi.mock('@/lib/core/utils/background', () => ({
23-
runDetached: (_label: string, work: () => Promise<unknown>) => {
24-
void work()
23+
vi.mock('@/lib/core/config/env', () => ({
24+
env: {
25+
get TRIGGER_SECRET_KEY() {
26+
return envState.secret
27+
},
2528
},
2629
}))
30+
vi.mock('@/lib/core/config/env-flags', () => ({
31+
get isTriggerDevEnabled() {
32+
return envState.triggerEnabled
33+
},
34+
}))
35+
vi.mock('@/lib/core/outbox/service', () => ({
36+
continueOutboxHandler: (reason: string) => ({
37+
outcome: 'deferred',
38+
reason,
39+
consumeAttempt: false,
40+
}),
41+
withOutboxHandlerTimeout: (handler: unknown, timeoutMs: number) =>
42+
Object.assign(handler as object, { timeoutMs }),
43+
}))
2744

2845
import {
2946
enqueueProjectionSourceAclBackfill,
@@ -32,6 +49,9 @@ import {
3249
} from '@/lib/knowledge/search/projection-source-acl-backfill'
3350

3451
const connection = { end: mockEnd }
52+
const handler =
53+
projectionSourceAclBackfillOutboxHandlers['knowledge.projection.source_acl.backfill']
54+
const context = { eventId: 'e', eventType: 'knowledge.projection.source_acl.backfill' }
3555

3656
describe('runProjectionSourceAclBackfill', () => {
3757
beforeEach(() => {
@@ -92,7 +112,7 @@ describe('runProjectionSourceAclBackfill', () => {
92112
})
93113
})
94114

95-
describe('enqueueProjectionSourceAclBackfill', () => {
115+
describe('the outbox event the migration leaves behind', () => {
96116
beforeEach(() => {
97117
vi.clearAllMocks()
98118
mockPostgres.mockReturnValue(connection)
@@ -106,26 +126,39 @@ describe('enqueueProjectionSourceAclBackfill', () => {
106126
})
107127

108128
it('hands the backfill to the Trigger.dev worker when there is one', async () => {
109-
await expect(enqueueProjectionSourceAclBackfill({ pageSize: 25 }, true)).resolves.toEqual({
129+
envState.triggerEnabled = true
130+
envState.secret = 'tr_secret'
131+
await expect(enqueueProjectionSourceAclBackfill({ pageSize: 25 })).resolves.toEqual({
110132
runId: 'run-1',
111133
})
112134
expect(mockTasksTrigger).toHaveBeenCalledWith(
113135
'projection-source-acl-backfill',
114136
{ pageSize: 25 },
115137
{ region: 'us-east-1' }
116138
)
139+
await expect(handler({}, context as never)).resolves.toBeUndefined()
140+
expect(mockTasksTrigger).toHaveBeenCalledTimes(2)
117141
expect(mockBackfill).not.toHaveBeenCalled()
118142
})
119143

120-
it('fills the projections detached in this process without one', async () => {
121-
await expect(enqueueProjectionSourceAclBackfill({}, false)).resolves.toBeNull()
144+
it('runs a bounded slice per outbox run without one, and stays pending until it is filled', async () => {
145+
envState.triggerEnabled = false
146+
envState.secret = undefined
147+
mockBackfill.mockResolvedValueOnce({
148+
projection: 'embedding_search',
149+
scanned: 100,
150+
written: 100,
151+
afterId: 'chunk-100',
152+
done: false,
153+
})
154+
await expect(handler({}, context as never)).resolves.toMatchObject({
155+
outcome: 'deferred',
156+
consumeAttempt: false,
157+
})
122158
expect(mockTasksTrigger).not.toHaveBeenCalled()
123-
await vi.waitFor(() => expect(mockBackfill).toHaveBeenCalledTimes(2))
124-
})
125-
126-
it('starts from the outbox event the migration leaves behind', () => {
127-
expect(Object.keys(projectionSourceAclBackfillOutboxHandlers)).toEqual([
128-
'knowledge.projection.source_acl.backfill',
129-
])
159+
expect(mockBackfill).toHaveBeenCalledTimes(1)
160+
expect(mockBackfill.mock.calls[0][2].budgetMs).toBeLessThanOrEqual(handler.timeoutMs!)
161+
await expect(handler({}, context as never)).resolves.toBeUndefined()
162+
expect(mockBackfill).toHaveBeenCalledTimes(3)
130163
})
131164
})

‎apps/sim/lib/knowledge/search/projection-source-acl-backfill.ts‎

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,13 @@ import postgres from 'postgres'
1010
import { resolveTriggerRegion } from '@/lib/core/async-jobs/region'
1111
import { env } from '@/lib/core/config/env'
1212
import { isTriggerDevEnabled } from '@/lib/core/config/env-flags'
13-
import type { OutboxHandlerRegistry } from '@/lib/core/outbox/service'
14-
import { runDetached } from '@/lib/core/utils/background'
13+
import {
14+
continueOutboxHandler,
15+
type DeferredOutboxHandlerResult,
16+
type OutboxEventContext,
17+
type OutboxHandlerRegistry,
18+
withOutboxHandlerTimeout,
19+
} from '@/lib/core/outbox/service'
1520

1621
const logger = createLogger('ProjectionSourceAclBackfill')
1722

@@ -77,34 +82,56 @@ export async function runProjectionSourceAclBackfill(
7782
}
7883
}
7984

85+
/** Whether the deployment has a Trigger.dev worker to hand the backfill to. */
86+
export function projectionSourceAclBackfillUsesTrigger(): boolean {
87+
return Boolean(isTriggerDevEnabled && env.TRIGGER_SECRET_KEY)
88+
}
89+
8090
/**
8191
* Starts the backfill the way the table backfill is started: on the deployment's Trigger.dev
82-
* worker when there is one, where bounded runs chain until both projections are filled, and
83-
* detached in this process otherwise. Safe to call again at any time — a run only fills rows still
84-
* unset.
92+
* worker, where bounded runs chain until both projections are filled. Safe to call again at any
93+
* time — a run only fills rows still unset.
8594
*/
8695
export async function enqueueProjectionSourceAclBackfill(
87-
payload: ProjectionSourceAclBackfillPayload = {},
88-
useTrigger = Boolean(isTriggerDevEnabled && env.TRIGGER_SECRET_KEY)
89-
): Promise<{ runId: string } | null> {
90-
if (useTrigger) {
91-
const { tasks } = await import('@trigger.dev/sdk')
92-
const handle = await tasks.trigger(PROJECTION_SOURCE_ACL_BACKFILL_TASK_ID, payload, {
93-
region: await resolveTriggerRegion(),
94-
})
95-
logger.info('Projection source and ACL backfill enqueued', { runId: handle.id })
96-
return { runId: handle.id }
97-
}
98-
runDetached(PROJECTION_SOURCE_ACL_BACKFILL_TASK_ID, () => runProjectionSourceAclBackfill(payload))
99-
return null
96+
payload: ProjectionSourceAclBackfillPayload = {}
97+
): Promise<{ runId: string }> {
98+
const { tasks } = await import('@trigger.dev/sdk')
99+
const handle = await tasks.trigger(PROJECTION_SOURCE_ACL_BACKFILL_TASK_ID, payload, {
100+
region: await resolveTriggerRegion(),
101+
})
102+
logger.info('Projection source and ACL backfill enqueued', { runId: handle.id })
103+
return { runId: handle.id }
100104
}
101105

106+
/** One outbox run's share of the backfill on a deployment without a worker, inside its window. */
107+
const OUTBOX_RUN_BUDGET_MS = 4 * 60 * 1000
108+
const OUTBOX_HANDLER_TIMEOUT_MS = 5 * 60 * 1000
109+
102110
/**
103-
* The event script migration `0021_embedding_search_connector` leaves behind, so the backfill it
104-
* hands off starts once the app that ships this handler is up, on every deployment.
111+
* Handles the event script migration `0021_embedding_search_connector` leaves behind, so the
112+
* backfill starts once the app that ships this handler is up, on every deployment. With a
113+
* Trigger.dev worker the event is done once the task is enqueued: the task owns its own retries
114+
* and continuations. Without one the backfill runs here, one bounded slice per outbox run, and the
115+
* event stays pending until both projections are filled — every slice's writes are durable, and a
116+
* slice that starts over after a restart skips the filled rows through the unfilled index, so an
117+
* interrupted deployment loses nothing but the time of one slice.
105118
*/
106119
export const projectionSourceAclBackfillOutboxHandlers: OutboxHandlerRegistry = {
107-
[PROJECTION_SOURCE_ACL_BACKFILL_OUTBOX_EVENT]: async () => {
108-
await enqueueProjectionSourceAclBackfill()
109-
},
120+
[PROJECTION_SOURCE_ACL_BACKFILL_OUTBOX_EVENT]: withOutboxHandlerTimeout(
121+
async (
122+
_payload: unknown,
123+
_context: OutboxEventContext
124+
): Promise<undefined | DeferredOutboxHandlerResult> => {
125+
if (projectionSourceAclBackfillUsesTrigger()) {
126+
await enqueueProjectionSourceAclBackfill()
127+
return undefined
128+
}
129+
const cursor = await runProjectionSourceAclBackfill({}, { budgetMs: OUTBOX_RUN_BUDGET_MS })
130+
if (!cursor) return undefined
131+
return continueOutboxHandler(
132+
`projection source and ACL backfill paused at ${cursor.projection} after ${cursor.afterId}`
133+
)
134+
},
135+
OUTBOX_HANDLER_TIMEOUT_MS
136+
),
110137
}

‎apps/sim/scripts/backfill-projection-source-acl.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,18 @@
1414

1515
import { createLogger } from '@sim/logger'
1616
import { toError } from '@sim/utils/errors'
17-
import { env } from '@/lib/core/config/env'
18-
import { isTriggerDevEnabled } from '@/lib/core/config/env-flags'
1917
import {
2018
enqueueProjectionSourceAclBackfill,
19+
projectionSourceAclBackfillUsesTrigger,
2120
runProjectionSourceAclBackfill,
2221
} from '@/lib/knowledge/search/projection-source-acl-backfill'
2322

2423
const logger = createLogger('BackfillProjectionSourceAcl')
2524

2625
async function main(): Promise<void> {
27-
if (isTriggerDevEnabled && env.TRIGGER_SECRET_KEY) {
28-
const handle = await enqueueProjectionSourceAclBackfill({}, true)
29-
logger.info('Backfill enqueued on the Trigger.dev worker', handle ?? {})
26+
if (projectionSourceAclBackfillUsesTrigger()) {
27+
const handle = await enqueueProjectionSourceAclBackfill()
28+
logger.info('Backfill enqueued on the Trigger.dev worker', handle)
3029
return
3130
}
3231
await runProjectionSourceAclBackfill({})

‎packages/db/script-migrations/0021_embedding_search_connector.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,14 +201,17 @@ export async function backfillProjectionSourceAcl(
201201
* vector projection the source index that lets the planner lead with a few sources when the
202202
* caller's tokens alone would match most of the index. Built concurrently, so the triggers and the
203203
* backfill keep writing. `CONCURRENTLY` cannot run in a transaction, and the pool's lock timeout
204-
* would cancel a build that merely waits for a long transaction to finish.
204+
* would cancel a build that merely waits for a long transaction to finish. The timeout is a
205+
* session setting, so one connection is reserved for it, the builds, and the reset — a pool
206+
* would otherwise hand the builds to connections that never saw the setting.
205207
*
206208
* The unfilled index on each projection lists the rows the backfill has not reached: each page
207209
* reads its rows from it instead of walking past every filled one, and the on-row predicate's
208210
* unfilled branch, an `OR` beside the ACL overlap, stays an index probe for the planner — once the
209211
* projection is filled, a probe of an empty index.
210212
*/
211-
export async function indexProjectionAcl(sql: Sql): Promise<void> {
213+
export async function indexProjectionAcl(pool: Sql): Promise<void> {
214+
const sql = await pool.reserve()
212215
const [{ timeout }] = await sql`SELECT current_setting('lock_timeout') AS timeout`
213216
await sql.unsafe('SET lock_timeout = 0')
214217
try {
@@ -237,14 +240,15 @@ export async function indexProjectionAcl(sql: Sql): Promise<void> {
237240
for (const projection of PROJECTION_SOURCE_ACL_TABLES) await sql.unsafe(`ANALYZE ${projection}`)
238241
} finally {
239242
await sql`SELECT set_config('lock_timeout', ${timeout}, false)`
243+
sql.release()
240244
}
241245
}
242246

243247
/**
244248
* Leaves the app one outbox event to start the backfill from. The outbox processor runs on every
245249
* deployment, so the backfill starts on its own once the app that ships the handler is up —
246-
* on the Trigger.dev worker where there is one, detached in the app otherwise — without an
247-
* operator remembering to. Idempotent under its fixed id.
250+
* enqueued on the Trigger.dev worker where there is one, run in bounded slices by the outbox
251+
* itself otherwise — without an operator remembering to. Idempotent under its fixed id.
248252
*/
249253
export async function enqueueProjectionSourceAclBackfillEvent(sql: Sql): Promise<void> {
250254
await sql`

0 commit comments

Comments
 (0)