Skip to content

Commit cd54396

Browse files
committed
fix(execution): finish the removal sweep's edge cases
- The removal stop handler loads both job queues independently and sweeps whichever loaded, so a primary queue that fails to load no longer skips the database queue's sweep. The failure is still rethrown for the outbox to retry. - A claimed resume is found under a parent log row in the legacy `paused` status too, one of the statuses a resume claims from. - The database queue re-checks for a matching active job after a short batch, since a selected job that finished before the update reached it shrinks the batch while later jobs still match. - The ban's completion log no longer reports its runs stopped when recording their stops failed.
1 parent 197e901 commit cd54396

7 files changed

Lines changed: 66 additions & 12 deletions

File tree

‎apps/sim/lib/core/async-jobs/backends/database.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { asyncJobs } from '@sim/db'
22
import {
33
dbChainMockFns,
44
flattenMockConditions,
5+
queueTableRows,
56
resetDbChainMock,
67
} from '@sim/testing/mocks/database.mock'
78
import { sleep } from '@sim/utils/helpers'
@@ -543,6 +544,15 @@ describe('DatabaseJobQueue workspace cancellation', () => {
543544
).resolves.toEqual({ cancelledJobs: 230, complete: true })
544545
})
545546

547+
it('reports a short batch incomplete while matching jobs remain', async () => {
548+
dbChainMockFns.returning.mockResolvedValueOnce(ids(99))
549+
queueTableRows(asyncJobs, [{ id: 'job-100' }])
550+
551+
await expect(
552+
new DatabaseJobQueue().cancelByWorkspace('workspace-1', { maxRuns: 1000 })
553+
).resolves.toEqual({ cancelledJobs: 99, complete: false })
554+
})
555+
546556
it('stops at its run cap and reports the sweep incomplete', async () => {
547557
dbChainMockFns.returning.mockResolvedValueOnce(ids(100)).mockResolvedValueOnce(ids(50, 100))
548558

‎apps/sim/lib/core/async-jobs/backends/database.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,21 @@ export class DatabaseJobQueue implements JobQueueBackend {
561561
for (const { id } of cancelled) abortInlineJob(id)
562562
cancelledJobs += cancelled.length
563563
if (cancelled.length < batchSize) {
564-
logger.info('Cancelled database jobs for workspace', { workspaceId, cancelledJobs })
565-
return { cancelledJobs, complete: true }
564+
/**
565+
* A short batch is not proof the sweep is done: a selected job that finished before the
566+
* update reached it shrinks the batch while later jobs may still match.
567+
*/
568+
const [remaining] = await db
569+
.select({ id: asyncJobs.id })
570+
.from(asyncJobs)
571+
.where(active)
572+
.limit(1)
573+
logger.info('Cancelled database jobs for workspace', {
574+
workspaceId,
575+
cancelledJobs,
576+
complete: !remaining,
577+
})
578+
return { cancelledJobs, complete: !remaining }
566579
}
567580
}
568581

‎apps/sim/lib/execution/removal-stop-records.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ export async function collectExecutionStops(
103103
eq(resumeQueue.status, 'claimed')
104104
)
105105
)
106-
.where(and(inArray(workflowExecutionLogs.status, ['running', 'pending']), inScope))
106+
.where(and(inArray(workflowExecutionLogs.status, ['running', 'pending', 'paused']), inScope))
107107

108108
const executions = new Map<string, PlannedExecutionStop>()
109109
for (const row of [...running, ...resuming]) {

‎apps/sim/lib/execution/removal-stops.integration.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ async function seedWorkflow(userId: string, workspaceId: string) {
103103
async function seedExecution(
104104
workspaceId: string,
105105
workflowId: string,
106-
status: 'running' | 'pending' = 'running'
106+
status: 'running' | 'pending' | 'paused' = 'running'
107107
) {
108108
const snapshotId = generateId()
109109
snapshotIds.push(snapshotId)
@@ -131,8 +131,12 @@ async function seedExecution(
131131
}
132132

133133
/** A paused run whose resume a worker has claimed and is executing under its own id. */
134-
async function seedActiveResume(workspaceId: string, workflowId: string) {
135-
const parentExecutionId = await seedExecution(workspaceId, workflowId, 'pending')
134+
async function seedActiveResume(
135+
workspaceId: string,
136+
workflowId: string,
137+
parentStatus: 'pending' | 'paused' = 'pending'
138+
) {
139+
const parentExecutionId = await seedExecution(workspaceId, workflowId, parentStatus)
136140
const pausedExecutionId = generateId()
137141
await db.insert(pausedExecutions).values({
138142
id: pausedExecutionId,
@@ -246,6 +250,11 @@ describe.runIf(Boolean(redisUrl))('removals stop the workflow runs they orphan',
246250
const doomedWorkflowId = await seedWorkflow(departingId, doomedWorkspaceId)
247251
const running = await seedExecution(doomedWorkspaceId, doomedWorkflowId)
248252
const resuming = await seedActiveResume(doomedWorkspaceId, doomedWorkflowId)
253+
const resumingLegacyPaused = await seedActiveResume(
254+
doomedWorkspaceId,
255+
await seedWorkflow(departingId, doomedWorkspaceId),
256+
'paused'
257+
)
249258
const queued = await seedQueuedJob(doomedWorkspaceId)
250259
const unrelatedJob = await seedQueuedJob(doomedWorkspaceId, 'cleanup-logs')
251260
const keptWorkspaceId = await seedWorkspace(bystanderId)
@@ -259,6 +268,7 @@ describe.runIf(Boolean(redisUrl))('removals stop the workflow runs they orphan',
259268

260269
await expect.poll(() => isCancelled(running)).toBe(true)
261270
await expect.poll(() => isCancelled(resuming)).toBe(true)
271+
await expect.poll(() => isCancelled(resumingLegacyPaused)).toBe(true)
262272
const recorded = await stopEvents([doomedWorkspaceId])
263273
expect(recorded.map((event) => event.status)).toEqual(['pending'])
264274

‎apps/sim/lib/execution/removal-stops.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* - Redis refuses the cancellation write → retry, and the queue sweep still runs
55
* - no Redis at all → the process-local flag is all there is, so the stop completes
66
* - a queue sweep fails → retry
7+
* - one queue fails to load → the other is still swept, then retry
78
* - a sweep stops at its cap having cancelled runs → continue without spending an attempt
89
* - a sweep stops at its cap having cancelled nothing → retry, so it cannot loop forever
910
*/
@@ -65,6 +66,13 @@ describe('execution removal stop outcome', () => {
6566
await expect(stopRemovedExecutions(payload, context())).rejects.toThrow('provider unavailable')
6667
})
6768

69+
it('still sweeps the database queue when the primary queue fails to load, then retries', async () => {
70+
asyncJobsMockFns.mockGetJobQueue.mockRejectedValueOnce(new Error('queue unavailable'))
71+
72+
await expect(stopRemovedExecutions(payload, context())).rejects.toThrow('queue unavailable')
73+
expect(mockJobQueue.cancelByWorkspace).toHaveBeenCalledWith('workspace-1', expect.anything())
74+
})
75+
6876
it('continues without spending an attempt when the sweep stops at its cap after progress', async () => {
6977
mockJobQueue.cancelByWorkspace.mockResolvedValue({ cancelledJobs: 1000, complete: false })
7078

‎apps/sim/lib/execution/removal-stops.ts‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { db } from '@sim/db'
22
import { z } from 'zod'
33
import { getInlineJobQueue, getJobQueue } from '@/lib/core/async-jobs'
4+
import type { JobQueueBackend } from '@/lib/core/async-jobs/types'
45
import {
56
continueOutboxHandler,
67
type OutboxHandler,
@@ -29,10 +30,17 @@ const payloadSchema = z.object({
2930

3031
/**
3132
* Sweeps the primary queue and the database queue that inline webhook runs use even when
32-
* Trigger.dev is primary. Both are attempted before the first failure is rethrown.
33+
* Trigger.dev is primary. Each queue that loads is swept, even when the other fails to load or
34+
* to sweep, before the first failure is rethrown for the outbox to retry.
3335
*/
3436
async function sweepWorkspaceJobs(workspaceId: string, signal: AbortSignal) {
35-
const queues = new Set([await getJobQueue(), await getInlineJobQueue()])
37+
const resolved = await Promise.allSettled([getJobQueue(), getInlineJobQueue()])
38+
const queues = new Set<JobQueueBackend>()
39+
const failures: unknown[] = []
40+
for (const result of resolved) {
41+
if (result.status === 'fulfilled') queues.add(result.value)
42+
else failures.push(result.reason)
43+
}
3644
const results = await Promise.allSettled(
3745
[...queues].map((queue) =>
3846
queue.cancelByWorkspace(workspaceId, { maxRuns: SWEEP_MAX_RUNS_PER_ATTEMPT, signal })
@@ -41,10 +49,13 @@ async function sweepWorkspaceJobs(workspaceId: string, signal: AbortSignal) {
4149
let cancelledJobs = 0
4250
let complete = true
4351
for (const result of results) {
44-
if (result.status === 'rejected') throw result.reason
45-
cancelledJobs += result.value.cancelledJobs
46-
complete &&= result.value.complete
52+
if (result.status === 'rejected') failures.push(result.reason)
53+
else {
54+
cancelledJobs += result.value.cancelledJobs
55+
complete &&= result.value.complete
56+
}
4757
}
58+
if (failures.length > 0) throw failures[0]
4859
return { cancelledJobs, complete }
4960
}
5061

‎apps/sim/lib/workflows/lifecycle.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,12 +327,14 @@ export async function disableUserResources(userId: string): Promise<void> {
327327
* not load that delivery. Logged, not thrown: the ban and archives have committed, and a retry
328328
* would find no unarchived workspace to stop.
329329
*/
330+
let stopFailed = false
330331
const stopRuns = async (workspaceIds: string[]): Promise<void> => {
331332
if (workspaceIds.length === 0) return
332333
try {
333334
const executions = await db.transaction((tx) => recordExecutionStops(tx, { workspaceIds }))
334335
await signalExecutionStops(executions)
335336
} catch (error) {
337+
stopFailed = true
336338
logger.error(`[${requestId}] Failed to stop runs for banned user ${userId}`, {
337339
workspaceIds,
338340
error: getErrorMessage(error),
@@ -375,7 +377,7 @@ export async function disableUserResources(userId: string): Promise<void> {
375377
}
376378

377379
logger.info(
378-
`[${requestId}] Disabled resources for user ${userId}: archived ${ownedWorkspaces.length} workspaces, deleted API keys, stopped their runs`
380+
`[${requestId}] Disabled resources for user ${userId}: archived ${ownedWorkspaces.length} workspaces, deleted API keys, ${stopFailed ? 'failed to stop some of their runs' : 'stopped their runs'}`
379381
)
380382
}
381383

0 commit comments

Comments
 (0)