Skip to content

Commit e15d551

Browse files
committed
fix(knowledge): create a Sim Search source without holding a transaction across the nested use cases
The first connect held an advisory-lock transaction while the nested base and connector use cases queried the pool, which the transaction tripwire now refuses, so every first connect of a new source failed. Concurrent first connects are coalesced per workspace base and per source within the process and re-check before creating instead.
1 parent 67f068a commit e15d551

2 files changed

Lines changed: 49 additions & 47 deletions

File tree

apps/sim/lib/knowledge/application/sim-search.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ const workspaceContext = {
110110
const principal = { kind: 'session' as const, userId: 'user-1', sessionId: 'session-1' }
111111
const existingConnector = { knowledgeBaseId: 'kb-search', connectorId: 'connector-drive' }
112112

113-
/** The first lookup runs outside the setup lock and the second inside it. */
113+
/** The first lookup runs before the coalesced creation and the second inside it. */
114114
function queueConnectorLookups(...results: Array<typeof existingConnector | null>) {
115115
for (const result of results) {
116116
queueTableRows(knowledgeConnector, result ? [result] : [])
@@ -216,7 +216,7 @@ describe('connectSimSearchConnector', () => {
216216
}),
217217
})
218218
)
219-
expect(dbChainMockFns.transaction).toHaveBeenCalledTimes(1)
219+
expect(dbChainMockFns.transaction).not.toHaveBeenCalled()
220220
expect(result).toEqual({
221221
knowledgeBaseId: 'kb-new',
222222
connectorId: 'connector-new',
@@ -240,9 +240,10 @@ describe('connectSimSearchConnector', () => {
240240
expect(mocks.createKnowledgeBase).not.toHaveBeenCalled()
241241
})
242242

243-
it('reuses the connector another first connect created while it waited for the lock', async () => {
243+
it('reuses the connector another first connect created while it waited', async () => {
244244
mocks.resolvePermission.mockResolvedValue('admin')
245245
queueConnectorLookups(null, existingConnector)
246+
queueTableRows(knowledgeBase, [{ id: existingConnector.knowledgeBaseId }])
246247

247248
const result = await connectSimSearchConnector.execute({
248249
principal,

apps/sim/lib/knowledge/application/sim-search.ts

Lines changed: 45 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import { resolvePrincipalSubjectUserId } from '@sim/auth/principal'
22
import { db } from '@sim/db'
33
import { knowledgeBase, knowledgeConnector } from '@sim/db/schema'
4-
import { and, asc, eq, isNull, sql } from 'drizzle-orm'
4+
import { and, asc, eq, isNull } from 'drizzle-orm'
5+
import { coalesceLocally } from '@/lib/concurrency/singleflight'
56
import {
67
InsufficientWorkspacePermissionsError,
78
requireCurrentHumanRole,
89
} from '@/lib/core/application/workspace-authorization'
910
import { OrchestrationError } from '@/lib/core/orchestration/types'
10-
import type { DbOrTx } from '@/lib/db/types'
1111
import { requireKnowledgeMemberAccessAvailable } from '@/lib/knowledge/access/availability'
1212
import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/authorized-knowledge-use-case'
1313
import { startKnowledgeConnectorMemberEnrollment } from '@/lib/knowledge/application/connector-access'
@@ -29,8 +29,6 @@ const SIM_SEARCH_KNOWLEDGE_BASE_DESCRIPTION =
2929
'What each person can open in the sources they connected, searched as them.'
3030
/** Between runs the change feeds keep deletions and unshares fresh; the hourly run fills the rest. */
3131
const SIM_SEARCH_SYNC_INTERVAL_MINUTES = 60
32-
/** How long a first connect waits for another first connect of the same workspace to finish. */
33-
const SIM_SEARCH_SETUP_LOCK_TIMEOUT_MS = 10_000
3432

3533
export interface ConnectSimSearchConnectorInput {
3634
workspaceId: string
@@ -47,12 +45,8 @@ export interface ConnectSimSearchConnectorResult {
4745
url: string
4846
}
4947

50-
async function findSimSearchConnector(
51-
executor: DbOrTx,
52-
workspaceId: string,
53-
connectorType: string
54-
) {
55-
const [row] = await executor
48+
async function findSimSearchConnector(workspaceId: string, connectorType: string) {
49+
const [row] = await db
5650
.select({ knowledgeBaseId: knowledgeBase.id, connectorId: knowledgeConnector.id })
5751
.from(knowledgeConnector)
5852
.innerJoin(knowledgeBase, eq(knowledgeBase.id, knowledgeConnector.knowledgeBaseId))
@@ -72,8 +66,8 @@ async function findSimSearchConnector(
7266
return row ?? null
7367
}
7468

75-
async function findSimSearchKnowledgeBase(executor: DbOrTx, workspaceId: string) {
76-
const [row] = await executor
69+
async function findSimSearchKnowledgeBase(workspaceId: string) {
70+
const [row] = await db
7771
.select({ id: knowledgeBase.id })
7872
.from(knowledgeBase)
7973
.where(
@@ -118,11 +112,11 @@ async function requireSimSearchSetupAdmin(
118112
* fields when it has any; every connect after that only enrolls. The OAuth
119113
* completion queues the member run, so indexing starts on its own.
120114
*
121-
* The creating branch runs under a per-workspace advisory lock and re-checks
122-
* for the connector once it holds it: nothing in the schema keeps two
123-
* concurrent first connects from each creating a Sim Search base and a
124-
* connector of the same source, and the second would index the same
125-
* accounts twice.
115+
* The creating branch shares one creation per workspace base and per source
116+
* within the process and re-checks before creating: nothing in the schema
117+
* keeps two concurrent first connects from each creating a Sim Search base
118+
* and a connector of the same source. The nested use cases query the pool,
119+
* so this cannot hold a transaction across them.
126120
*/
127121
export const connectSimSearchConnector = defineAuthorizedKnowledgeUseCase({
128122
operation: knowledgeOperations.simSearchConnect,
@@ -137,7 +131,7 @@ export const connectSimSearchConnector = defineAuthorizedKnowledgeUseCase({
137131
)
138132
}
139133
const workspaceId = context.workspaceId
140-
let target = await findSimSearchConnector(db, workspaceId, input.connectorType)
134+
let target = await findSimSearchConnector(workspaceId, input.connectorType)
141135
if (!target) {
142136
const userId = resolvePrincipalSubjectUserId(principal)
143137
if (!userId) throw new OrchestrationError('forbidden', 'Sign in to connect your account')
@@ -157,17 +151,17 @@ export const connectSimSearchConnector = defineAuthorizedKnowledgeUseCase({
157151
requireKnowledgeMemberAccessAvailable({ workspaceId }),
158152
requireSimSearchSetupAdmin(userId, context, meta.name),
159153
])
160-
target = await db.transaction(async (tx) => {
161-
await tx.execute(
162-
sql`select set_config('lock_timeout', ${`${SIM_SEARCH_SETUP_LOCK_TIMEOUT_MS}ms`}, true)`
163-
)
164-
await tx.execute(
165-
sql`select pg_advisory_xact_lock(hashtextextended(${`sim-search:connect:${workspaceId}`}, 0))`
166-
)
167-
const existing = await findSimSearchConnector(tx, workspaceId, input.connectorType)
168-
if (existing) return existing
169-
const knowledgeBaseId =
170-
(await findSimSearchKnowledgeBase(tx, workspaceId))?.id ??
154+
/**
155+
* Concurrent first connects in this process share one creation per
156+
* workspace base and per source, and each re-checks before creating.
157+
* Two instances can still race in the same instant; both then converge
158+
* on the oldest row, which every lookup here orders by, and the stray
159+
* one is inert.
160+
*/
161+
const knowledgeBaseId = await coalesceLocally(
162+
`sim-search:base:${workspaceId}`,
163+
async () =>
164+
(await findSimSearchKnowledgeBase(workspaceId))?.id ??
171165
(
172166
await createKnowledgeBase.execute({
173167
principal,
@@ -180,21 +174,28 @@ export const connectSimSearchConnector = defineAuthorizedKnowledgeUseCase({
180174
request,
181175
})
182176
).knowledgeBase.id
183-
const created = await createKnowledgeConnector.execute({
184-
principal,
185-
input: {
186-
knowledgeBaseId,
187-
assertedWorkspaceId: workspaceId,
188-
connectorType: input.connectorType,
189-
sourceConfig,
190-
syncIntervalMinutes: SIM_SEARCH_SYNC_INTERVAL_MINUTES,
191-
accessMode: 'members',
192-
source: 'ui',
193-
},
194-
request,
195-
})
196-
return { knowledgeBaseId, connectorId: created.connector.id }
197-
})
177+
)
178+
target = await coalesceLocally(
179+
`sim-search:connect:${workspaceId}:${input.connectorType}`,
180+
async () => {
181+
const existing = await findSimSearchConnector(workspaceId, input.connectorType)
182+
if (existing) return existing
183+
const created = await createKnowledgeConnector.execute({
184+
principal,
185+
input: {
186+
knowledgeBaseId,
187+
assertedWorkspaceId: workspaceId,
188+
connectorType: input.connectorType,
189+
sourceConfig,
190+
syncIntervalMinutes: SIM_SEARCH_SYNC_INTERVAL_MINUTES,
191+
accessMode: 'members',
192+
source: 'ui',
193+
},
194+
request,
195+
})
196+
return { knowledgeBaseId, connectorId: created.connector.id }
197+
}
198+
)
198199
}
199200
const { url } = await startKnowledgeConnectorMemberEnrollment.execute({
200201
principal,

0 commit comments

Comments
 (0)