Skip to content

Commit 420c0df

Browse files
authored
fix(knowledge): settle detach reservations when a knowledge base is purged (#8184)
* fix(knowledge): settle detach reservations when a knowledge base is purged - Retention purge settles a detached connector's remaining detach_reserved_bytes (same lock order and settlement branches as the detach job) and zeroes it before the knowledge base delete cascades the connector away - Test drain helper picks the oldest pending outbox row - Unfilled-projection fixture writes its Tin row the way the projection trigger does - Knowledge ACL harness also runs the 0021 projection source/ACL postgres test * fix(knowledge): settle a purged base's detach reservations as one net amount * fix(knowledge): settle a purged base's reservations before deleting its documents * test(memory): always change a byte when tampering with a checkpoint's auth tag * fix(knowledge): settle overdrawn reservations before a purge's documents and the rest after * fix(knowledge): pause a connector detach while its knowledge base is deleted
1 parent cda0148 commit 420c0df

10 files changed

Lines changed: 625 additions & 34 deletions

File tree

‎.github/workflows/test-build.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ jobs:
278278
lib/knowledge/__integration__/slack-empty-threads.integration.ts
279279
lib/knowledge/__integration__/kb-block-search.integration.ts
280280
lib/knowledge/__integration__/unfilled-projection-source.integration.ts
281+
lib/knowledge/__integration__/purged-detach-reservation.integration.ts
281282
lib/core/outbox/service.integration.ts
282283
lib/knowledge/__integration__/connector-upload.integration.ts
283284
lib/uploads/contexts/organization-logo/application.integration.ts

‎apps/sim/background/cleanup-soft-deletes.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const {
2626
mockPrepareChatCleanup,
2727
mockResolveStorageBillingContext,
2828
mockSelectRowsByIdChunks,
29+
mockSettleDetachedConnectorReservations,
2930
mockDeduplicateWorkflowName,
3031
mockAllocateUniqueWorkspaceFileName,
3132
mockDeduplicateFolderName,
@@ -46,6 +47,7 @@ const {
4647
mockPrepareChatCleanup: vi.fn(async () => ({ execute: vi.fn(async () => undefined) })),
4748
mockResolveStorageBillingContext: vi.fn(),
4849
mockSelectRowsByIdChunks: vi.fn(async () => [] as unknown[]),
50+
mockSettleDetachedConnectorReservations: vi.fn(async () => undefined),
4951
}))
5052

5153
vi.mock('@/lib/billing/cleanup-dispatcher', () => ({ runCleanupWithLimits: vi.fn() }))
@@ -71,6 +73,10 @@ vi.mock('@/lib/billing/storage', () => ({
7173
resolveStorageBillingContext: mockResolveStorageBillingContext,
7274
}))
7375

76+
vi.mock('@/lib/knowledge/connectors/detachment', () => ({
77+
settleDetachedConnectorReservations: mockSettleDetachedConnectorReservations,
78+
}))
79+
7480
vi.mock('@/lib/knowledge/documents/service', () => ({
7581
hardDeleteDocuments: mockHardDeleteDocuments,
7682
}))
@@ -303,6 +309,32 @@ describe('cleanup soft deletes', () => {
303309
)
304310
})
305311

312+
it('settles overdrawn reservations before the documents and the rest before the base delete', async () => {
313+
mockChunkedBatchDelete.mockImplementationOnce(
314+
async (options: { onBatch?: (rows: Array<{ id: string }>) => Promise<void> }) => {
315+
await options.onBatch?.([{ id: 'kb-1' }, { id: 'kb-2' }])
316+
mockKnowledgeBaseContainerDelete()
317+
return { deleted: 2, failed: 0 }
318+
}
319+
)
320+
dbChainMockFns.limit
321+
.mockResolvedValueOnce([{ id: 'doc-1' }])
322+
.mockResolvedValueOnce([])
323+
.mockResolvedValueOnce([])
324+
325+
await runCleanupSoftDeletes(basePayload)
326+
327+
expect(mockSettleDetachedConnectorReservations.mock.calls).toEqual([
328+
[['kb-1', 'kb-2'], 'overdrawn'],
329+
[['kb-1', 'kb-2'], 'remaining'],
330+
])
331+
const [overdrawn, remaining] = mockSettleDetachedConnectorReservations.mock.invocationCallOrder
332+
const [deletedDocuments] = mockHardDeleteDocuments.mock.invocationCallOrder
333+
expect(overdrawn).toBeLessThan(deletedDocuments)
334+
expect(deletedDocuments).toBeLessThan(remaining)
335+
expect(remaining).toBeLessThan(mockKnowledgeBaseContainerDelete.mock.invocationCallOrder[0])
336+
})
337+
306338
it('soft-deletes abandoned KB bindings and removes their storage objects', async () => {
307339
dbChainMockFns.limit
308340
.mockResolvedValueOnce([{ key: 'kb/orphan-1' }, { key: 'kb/orphan-2' }])

‎apps/sim/background/cleanup-soft-deletes.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {
4040
resolveCleanupOwnerScope,
4141
} from '@/lib/cleanup/resource-scope'
4242
import { deduplicateFolderName } from '@/lib/folders/naming'
43+
import { settleDetachedConnectorReservations } from '@/lib/knowledge/connectors/detachment'
4344
import { hardDeleteDocuments } from '@/lib/knowledge/documents/service'
4445
import type { StorageContext } from '@/lib/uploads'
4546
import { isUsingCloudStorage, StorageService } from '@/lib/uploads'
@@ -445,11 +446,17 @@ async function cleanupExpiredKnowledgeBases(
445446
isNotNull(knowledgeBase.deletedAt),
446447
lt(knowledgeBase.deletedAt, retentionDate)
447448
),
448-
onBatch: (rows: { id: string }[]) =>
449-
hardDeleteKnowledgeBaseDocuments(
450-
rows.map(({ id }) => id),
451-
label
452-
),
449+
/**
450+
* The bases' DELETE cascades their connectors away, so a detached connector's reservation is
451+
* settled here: an overdrawn one before the documents (their deletion floors usage at zero),
452+
* the rest after them, so a deletion that fails partway leaves every step's ledger consistent.
453+
*/
454+
onBatch: async (rows: { id: string }[]) => {
455+
const knowledgeBaseIds = rows.map(({ id }) => id)
456+
await settleDetachedConnectorReservations(knowledgeBaseIds, 'overdrawn')
457+
await hardDeleteKnowledgeBaseDocuments(knowledgeBaseIds, label)
458+
await settleDetachedConnectorReservations(knowledgeBaseIds, 'remaining')
459+
},
453460
}
454461
return scope.kind === 'workspace'
455462
? chunkedBatchDelete({ ...options, workspaceIds: scope.ids })

‎apps/sim/lib/knowledge/__integration__/drain-connector-event.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { db } from '@sim/db'
22
import { outboxEvent } from '@sim/db/schema'
3-
import { and, eq, sql } from 'drizzle-orm'
3+
import { and, asc, eq, sql } from 'drizzle-orm'
44
import { expect } from 'vitest'
55
import { processOutboxEventById } from '@/lib/core/outbox/service'
66
import { knowledgeDocumentProcessingOutboxHandlers } from '@/lib/knowledge/documents/processing-outbox-handler'
@@ -13,9 +13,11 @@ export async function drainConnectorEvent(connectorId: string, eventType: string
1313
.where(
1414
and(
1515
eq(outboxEvent.eventType, eventType),
16+
eq(outboxEvent.status, 'pending'),
1617
sql`${outboxEvent.payload}->>'connectorId' = ${connectorId}`
1718
)
1819
)
20+
.orderBy(asc(outboxEvent.availableAt), asc(outboxEvent.id))
1921
.limit(1)
2022
expect(job).toBeDefined()
2123
let status = await processOutboxEventById(job.id, knowledgeDocumentProcessingOutboxHandlers)

0 commit comments

Comments
 (0)