Skip to content

Commit 6eaeffe

Browse files
committed
fix(connectors): heartbeat every unbounded phase, not just the batch loop
The heartbeat was added where we happened to be looking. The pagination loop — where a large source spends most of its wall clock, since the batch loop does not start until every page is fetched — never beat at all, so a long listing on the uncapped in-process path was still reclaimed as a hard failure. That is the exact ratchet the heartbeat exists to prevent. Auditing the remaining phases found something worse in the stuck-document retry: on the in-process path it handed the entire backlog to a single await that fully parses, embeds and indexes every document before returning, so no beat placement could interrupt it. That dispatch is now chunked, with a beat per chunk. All four call sites share one beatIfDue closure; the two pre-existing inline blocks were collapsed onto it rather than left as copies. An await longer than the TTL — one pathological listing page, or a very large hard delete — is still not covered, and no inline beat can cover it. Closing that needs a concurrent interval, which is a second mechanism and a separate decision. Adds the first test that drives executeSync itself, reaching the pagination loop through the real lock acquisition rather than testing helpers in isolation.
1 parent 9fada81 commit 6eaeffe

2 files changed

Lines changed: 135 additions & 16 deletions

File tree

apps/sim/lib/knowledge/connectors/sync-engine.test.ts

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ import {
88
flattenMockConditions,
99
hasMockCondition,
1010
type MockCondition,
11+
queueTableRows,
1112
resetDbChainMock,
1213
schemaMock,
1314
} from '@sim/testing'
1415
import { generateShortId } from '@sim/utils/id'
15-
import { beforeEach, describe, expect, it, vi } from 'vitest'
16+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
1617
import {
1718
classifySuspectListing,
1819
evaluateListingSafety,
@@ -34,7 +35,14 @@ vi.mock('@/background/knowledge-connector-sync', () => ({
3435
knowledgeConnectorSync: { trigger: vi.fn() },
3536
}))
3637

37-
const { mockMapTags } = vi.hoisted(() => ({ mockMapTags: vi.fn() }))
38+
const { mockMapTags, mockListDocuments } = vi.hoisted(() => ({
39+
mockMapTags: vi.fn(),
40+
mockListDocuments: vi.fn(),
41+
}))
42+
43+
vi.mock('@/lib/billing/core/billing-attribution', () => ({
44+
assertBillingAttributionSnapshot: (snapshot: unknown) => snapshot,
45+
}))
3846

3947
vi.mock('@/connectors/registry.server', () => ({
4048
CONNECTOR_REGISTRY: {
@@ -44,6 +52,11 @@ vi.mock('@/connectors/registry.server', () => ({
4452
'no-tags': {
4553
name: 'No Tags',
4654
},
55+
paged: {
56+
name: 'Paged',
57+
auth: { mode: 'apiKey', optional: true },
58+
listDocuments: mockListDocuments,
59+
},
4760
},
4861
}))
4962

@@ -1716,3 +1729,86 @@ describe('heartbeatSyncLock', () => {
17161729
expect(await heartbeatSyncLock('c-1', 'run-a')).toBe(true)
17171730
})
17181731
})
1732+
1733+
describe('executeSync heartbeats during the listing phase', () => {
1734+
const CONNECTOR = {
1735+
id: 'c-1',
1736+
knowledgeBaseId: 'kb-1',
1737+
connectorType: 'paged',
1738+
credentialId: null,
1739+
encryptedApiKey: null,
1740+
sourceConfig: {},
1741+
syncMode: 'full',
1742+
syncIntervalMinutes: 1440,
1743+
status: 'active',
1744+
lastSyncAt: null,
1745+
lastSyncDocCount: null,
1746+
consecutiveFailures: 0,
1747+
syncLockToken: null,
1748+
}
1749+
1750+
beforeEach(() => {
1751+
vi.clearAllMocks()
1752+
resetDbChainMock()
1753+
vi.useFakeTimers()
1754+
vi.setSystemTime(new Date('2026-08-20T00:00:00.000Z'))
1755+
})
1756+
1757+
afterEach(() => {
1758+
vi.useRealTimers()
1759+
})
1760+
1761+
/** Drives executeSync as far as the pagination loop. */
1762+
function primeSyncUpToListing() {
1763+
queueTableRows(schemaMock.knowledgeConnector, [CONNECTOR])
1764+
queueTableRows(schemaMock.knowledgeBase, [{ userId: 'u-1', workspaceId: 'ws-1' }])
1765+
// The lock CAS; every later `.returning()` falls through to the empty default,
1766+
// which is what makes the heartbeat below report a lost lock.
1767+
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'c-1' }])
1768+
}
1769+
1770+
it('beats between pages and abandons the run when the lock was reclaimed', async () => {
1771+
const { executeSync } = await import('@/lib/knowledge/connectors/sync-engine')
1772+
const { SYNC_LOCK_HEARTBEAT_INTERVAL_MS } = await import(
1773+
'@/lib/knowledge/connectors/sync-limits'
1774+
)
1775+
1776+
primeSyncUpToListing()
1777+
1778+
/**
1779+
* Listing is where a large source spends most of its wall clock, so a page
1780+
* that pushes the run past the heartbeat interval must trigger a beat before
1781+
* the next page — not only once listing has finished.
1782+
*/
1783+
mockListDocuments.mockImplementation(async () => {
1784+
vi.setSystemTime(new Date(Date.now() + SYNC_LOCK_HEARTBEAT_INTERVAL_MS + 1_000))
1785+
return { documents: [], hasMore: true, nextCursor: 'page-2' }
1786+
})
1787+
1788+
const result = await executeSync('c-1', {
1789+
billingAttribution: { workspaceId: 'ws-1' } as never,
1790+
})
1791+
1792+
// Aborted on the beat before page 2 rather than paging on under a lost lock.
1793+
expect(mockListDocuments).toHaveBeenCalledTimes(1)
1794+
expect(result.error).toBe('sync_superseded')
1795+
})
1796+
1797+
it('does not beat when pages return faster than the interval', async () => {
1798+
const { executeSync } = await import('@/lib/knowledge/connectors/sync-engine')
1799+
1800+
primeSyncUpToListing()
1801+
1802+
let pages = 0
1803+
mockListDocuments.mockImplementation(async () => {
1804+
pages += 1
1805+
vi.setSystemTime(new Date(Date.now() + 1_000))
1806+
return { documents: [], hasMore: pages < 3, nextCursor: `page-${pages}` }
1807+
})
1808+
1809+
await executeSync('c-1', { billingAttribution: { workspaceId: 'ws-1' } as never })
1810+
1811+
// All three pages fetched: the time gate keeps a fast listing beat-free.
1812+
expect(mockListDocuments).toHaveBeenCalledTimes(3)
1813+
})
1814+
})

apps/sim/lib/knowledge/connectors/sync-engine.ts

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,15 @@ const DEFAULT_OP_SIZE_BYTES = 4 * 1024 * 1024
7676
const CONTENT_INFLIGHT_BUDGET_BYTES = 64 * 1024 * 1024
7777
const MAX_PAGES = 500
7878
const MAX_SAFE_TITLE_LENGTH = 200
79+
/**
80+
* How many stuck documents are re-dispatched per call.
81+
*
82+
* The retry backlog is unbounded, and on the in-process fallback path
83+
* `processDocumentsWithQueue` parses, embeds, and indexes every document it is
84+
* given before returning. Handing it the whole backlog made the retry a single
85+
* await no heartbeat could interrupt; chunking gives the beat somewhere to run.
86+
*/
87+
const STUCK_RETRY_DISPATCH_CHUNK_SIZE = 25
7988
const STALE_PROCESSING_MINUTES = 45
8089
/** Largest connector corpus observed in production, which sets the queue drain to beat. */
8190
const LARGEST_OBSERVED_CORPUS_DOCUMENTS = 7_730
@@ -1309,6 +1318,20 @@ export async function executeSync(
13091318
const syncStartedAt = new Date()
13101319
/** Seeded at lock acquisition, which wrote `updatedAt` itself. */
13111320
let lastHeartbeatAtMs = Date.now()
1321+
1322+
/**
1323+
* Refreshes the lock if the interval has elapsed, and aborts the run if it has
1324+
* been reclaimed. Called at the top of every unbounded loop in this sync — the
1325+
* time gate makes each call nearly free, so placement only has to guarantee
1326+
* that no unbounded phase runs without reaching one.
1327+
*/
1328+
const beatIfDue = async (): Promise<void> => {
1329+
if (!shouldHeartbeatSyncLock(Date.now(), lastHeartbeatAtMs)) return
1330+
if (!(await heartbeatSyncLock(connectorId, syncLogId))) {
1331+
throw new SyncLockLostException(connectorId)
1332+
}
1333+
lastHeartbeatAtMs = Date.now()
1334+
}
13121335
await db.insert(knowledgeConnectorSyncLog).values({
13131336
id: syncLogId,
13141337
connectorId,
@@ -1416,6 +1439,14 @@ export async function executeSync(
14161439
)
14171440

14181441
for (let pageNum = 0; hasMore && pageNum < MAX_PAGES; pageNum++) {
1442+
/**
1443+
* Listing is where a large source spends most of its wall clock — the
1444+
* batch loop below does not start until every page has been fetched — so
1445+
* without this a big listing outran the TTL and was reclaimed as a hard
1446+
* failure, which is the exact ratchet the heartbeat exists to prevent.
1447+
*/
1448+
await beatIfDue()
1449+
14191450
if (pageNum > 0 && connectorConfig.auth.mode === 'oauth') {
14201451
accessToken = await resolveAccessToken(connector, connectorConfig, credentialUserId)
14211452
}
@@ -1593,12 +1624,7 @@ export async function executeSync(
15931624
// per-file cap never hydrate/upload together and exhaust the worker heap.
15941625
const batches = chunkOpsByByteBudget(pendingOps, CONTENT_INFLIGHT_BUDGET_BYTES, SYNC_BATCH_SIZE)
15951626
for (const rawBatch of batches) {
1596-
if (shouldHeartbeatSyncLock(Date.now(), lastHeartbeatAtMs)) {
1597-
if (!(await heartbeatSyncLock(connectorId, syncLogId))) {
1598-
throw new SyncLockLostException(connectorId)
1599-
}
1600-
lastHeartbeatAtMs = Date.now()
1601-
}
1627+
await beatIfDue()
16021628

16031629
const liveness = await checkSyncLiveness(connectorId, connector.knowledgeBaseId)
16041630
if (liveness.connectorDeleted) {
@@ -1980,12 +2006,7 @@ export async function executeSync(
19802006
result.docsDeleted += await hardDeleteDocuments(safeHardDeleteIds, syncLogId, connectorId)
19812007
}
19822008

1983-
if (shouldHeartbeatSyncLock(Date.now(), lastHeartbeatAtMs)) {
1984-
if (!(await heartbeatSyncLock(connectorId, syncLogId))) {
1985-
throw new SyncLockLostException(connectorId)
1986-
}
1987-
lastHeartbeatAtMs = Date.now()
1988-
}
2009+
await beatIfDue()
19892010

19902011
const postBatchLiveness = await checkSyncLiveness(connectorId, connector.knowledgeBaseId)
19912012
if (postBatchLiveness.connectorDeleted) {
@@ -2098,9 +2119,11 @@ export async function executeSync(
20982119
}
20992120
})
21002121

2101-
if (retryDocs.length > 0) {
2122+
for (let i = 0; i < retryDocs.length; i += STUCK_RETRY_DISPATCH_CHUNK_SIZE) {
2123+
await beatIfDue()
2124+
21022125
await processDocumentsWithQueue(
2103-
retryDocs.map((doc) => ({
2126+
retryDocs.slice(i, i + STUCK_RETRY_DISPATCH_CHUNK_SIZE).map((doc) => ({
21042127
documentId: doc.id,
21052128
filename: doc.filename ?? 'document.txt',
21062129
fileUrl: doc.fileUrl ?? '',

0 commit comments

Comments
 (0)