Skip to content

Commit f95af77

Browse files
fix(knowledge): serialize document processing attempts
1 parent a28b053 commit f95af77

4 files changed

Lines changed: 82 additions & 8 deletions

File tree

apps/sim/lib/knowledge/documents/document-processing-source.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ describe('knowledge document processing source', () => {
138138
.mockResolvedValueOnce([PERSISTED_CONTEXT])
139139
.mockResolvedValueOnce([PERSISTED_PROVENANCE_ROW])
140140
.mockResolvedValueOnce([{ id: 'document-1' }])
141+
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'document-1' }])
141142
mockCheckActorUsageLimits.mockResolvedValue({ isExceeded: false })
142143
mockGetFileMetadataByKeys.mockImplementation(async (_keys: string[], context: string) =>
143144
context === 'workspace' ? [SOURCE_BINDING] : []
@@ -228,4 +229,18 @@ describe('knowledge document processing source', () => {
228229
expect(mockProcessDocument).not.toHaveBeenCalled()
229230
expect(mockGenerateEmbeddings).not.toHaveBeenCalled()
230231
})
232+
233+
it('does not start work when another processing attempt already owns the document', async () => {
234+
dbChainMockFns.returning.mockReset().mockResolvedValueOnce([])
235+
236+
await processDocumentAsync('knowledge-base-1', 'document-1', {
237+
filename: 'stale.pdf',
238+
fileUrl: 'https://example.com/stale.pdf',
239+
fileSize: 1,
240+
mimeType: 'text/plain',
241+
})
242+
243+
expect(mockProcessDocument).not.toHaveBeenCalled()
244+
expect(mockGenerateEmbeddings).not.toHaveBeenCalled()
245+
})
231246
})

apps/sim/lib/knowledge/documents/processing-outbox-handler.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,19 @@ describe('knowledge document processing outbox handler', () => {
105105
}
106106
)
107107

108+
it('keeps the event retryable while an earlier processing attempt is active', async () => {
109+
mocks.getKnowledgeDocument.mockResolvedValueOnce({
110+
...DOCUMENT,
111+
processingStatus: 'processing',
112+
})
113+
114+
await expect(handler()(PAYLOAD, createContext())).rejects.toThrow(
115+
'Knowledge document document-1 is already being processed'
116+
)
117+
118+
expect(mocks.processDocumentsWithQueue).not.toHaveBeenCalled()
119+
})
120+
108121
it('propagates dispatch failures so the outbox schedules a retry', async () => {
109122
const failure = new Error('queue unavailable')
110123
mocks.processDocumentsWithQueue.mockRejectedValueOnce(failure)

apps/sim/lib/knowledge/documents/processing-outbox-handler.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ const processKnowledgeDocument: OutboxHandler<unknown> = async (rawPayload, cont
5959
context.signal.throwIfAborted()
6060
const document = await getKnowledgeDocument(payload.knowledgeBaseId, payload.documentId)
6161
if (!document || document.processingStatus === 'completed') return
62+
if (document.processingStatus === 'processing') {
63+
throw new Error(`Knowledge document ${document.id} is already being processed`)
64+
}
6265

6366
context.signal.throwIfAborted()
6467
await processDocumentsWithQueue(

apps/sim/lib/knowledge/documents/service.ts

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,7 @@ export async function processDocumentAsync(
778778
providedBillingContext?: BillingAttributionSnapshot | DocumentProcessingBillingContext
779779
): Promise<void> {
780780
const startTime = Date.now()
781+
const processingStartedAt = new Date()
781782
try {
782783
logger.info(`[${documentId}] Starting document processing`, {
783784
knowledgeBaseId,
@@ -815,6 +816,7 @@ export async function processDocumentAsync(
815816
boolean1: document.boolean1,
816817
boolean2: document.boolean2,
817818
boolean3: document.boolean3,
819+
processingStatus: document.processingStatus,
818820
})
819821
.from(document)
820822
.innerJoin(knowledgeBase, eq(knowledgeBase.id, document.knowledgeBaseId))
@@ -856,17 +858,30 @@ export async function processDocumentAsync(
856858
mimeType: ctx.mimeType,
857859
}
858860

859-
await db
861+
const [claimedDocument] = await db
860862
.update(document)
861863
.set({
862864
processingStatus: 'processing',
863-
processingStartedAt: new Date(),
865+
processingStartedAt,
864866
processingCompletedAt: null,
865867
processingError: null,
866868
})
867869
.where(
868-
and(eq(document.id, documentId), isNull(document.archivedAt), isNull(document.deletedAt))
870+
and(
871+
eq(document.id, documentId),
872+
inArray(document.processingStatus, ['pending', 'failed']),
873+
isNull(document.archivedAt),
874+
isNull(document.deletedAt)
875+
)
869876
)
877+
.returning({ id: document.id })
878+
879+
if (!claimedDocument) {
880+
logger.info(`[${documentId}] Skipping document processing because another attempt owns it`, {
881+
processingStatus: ctx.processingStatus,
882+
})
883+
return
884+
}
870885

871886
logger.info(`[${documentId}] Status updated to 'processing', starting document processor`)
872887

@@ -935,7 +950,13 @@ export async function processDocumentAsync(
935950
usageGate.message ?? 'Usage limit exceeded. Please upgrade your plan to continue.',
936951
processingCompletedAt: new Date(),
937952
})
938-
.where(eq(document.id, documentId))
953+
.where(
954+
and(
955+
eq(document.id, documentId),
956+
eq(document.processingStatus, 'processing'),
957+
eq(document.processingStartedAt, processingStartedAt)
958+
)
959+
)
939960
return
940961
}
941962
let billableEmbeddingTokens = 0
@@ -954,6 +975,7 @@ export async function processDocumentAsync(
954975
currentSourceFileProvenance
955976
)
956977

978+
let processingCommitted = false
957979
await withTimeout(
958980
runWithKnowledgeModelInputProvenance(
959981
documentSecretContext.registry,
@@ -1069,23 +1091,26 @@ export async function processDocumentAsync(
10691091
updatedAt: now,
10701092
}))
10711093

1072-
await db.transaction(async (tx) => {
1094+
processingCommitted = await db.transaction(async (tx) => {
10731095
const activeDocument = await tx
10741096
.select({ id: document.id })
10751097
.from(document)
10761098
.innerJoin(knowledgeBase, eq(document.knowledgeBaseId, knowledgeBase.id))
10771099
.where(
10781100
and(
10791101
eq(document.id, documentId),
1102+
eq(document.processingStatus, 'processing'),
1103+
eq(document.processingStartedAt, processingStartedAt),
10801104
isNull(document.archivedAt),
10811105
isNull(document.deletedAt),
10821106
isNull(knowledgeBase.deletedAt)
10831107
)
10841108
)
1109+
.for('update', { of: document })
10851110
.limit(1)
10861111

10871112
if (activeDocument.length === 0) {
1088-
return
1113+
return false
10891114
}
10901115

10911116
if (embeddingRecords.length > 0) {
@@ -1131,7 +1156,14 @@ export async function processDocumentAsync(
11311156
processingCompletedAt: now,
11321157
processingError: null,
11331158
})
1134-
.where(eq(document.id, documentId))
1159+
.where(
1160+
and(
1161+
eq(document.id, documentId),
1162+
eq(document.processingStatus, 'processing'),
1163+
eq(document.processingStartedAt, processingStartedAt)
1164+
)
1165+
)
1166+
return true
11351167
})
11361168
},
11371169
{
@@ -1144,6 +1176,11 @@ export async function processDocumentAsync(
11441176
'Document processing'
11451177
)
11461178

1179+
if (!processingCommitted) {
1180+
logger.info(`[${documentId}] Discarded output from an obsolete processing attempt`)
1181+
return
1182+
}
1183+
11471184
const processingTime = Date.now() - startTime
11481185
logger.info(`[${documentId}] Successfully processed document in ${processingTime}ms`)
11491186

@@ -1205,7 +1242,13 @@ export async function processDocumentAsync(
12051242
processingError: errorMessage,
12061243
processingCompletedAt: new Date(),
12071244
})
1208-
.where(eq(document.id, documentId))
1245+
.where(
1246+
and(
1247+
eq(document.id, documentId),
1248+
eq(document.processingStatus, 'processing'),
1249+
eq(document.processingStartedAt, processingStartedAt)
1250+
)
1251+
)
12091252

12101253
throw error
12111254
}

0 commit comments

Comments
 (0)