Skip to content

Commit 84d9164

Browse files
committed
fix(execution): do not lose the backlog or publish terminal status early
Draining the backlog ahead of the terminal event left the terminal status armed, so whichever chunk emptied the queue stamped the run complete before its terminal event was written — the inverse of the ordering the drain was added to guarantee. Disarm the status for the drain and restore it afterwards. The drain's result was also discarded: a transient Redis failure requeues its batch, and the unconditional reassignment that followed dropped those events even though the budget never rejected them. Keep whatever could not be persisted, and publish the terminal event alone only once nothing earlier is still queued — failing otherwise lets the caller degrade, which records the status without claiming the missing events arrived. Leave eventId unset on a failed resume-path write. Assigning 0 was persisted by clients as a reconnect cursor and rewound them to the start of the run.
1 parent 01f34e5 commit 84d9164

3 files changed

Lines changed: 66 additions & 11 deletions

File tree

apps/sim/lib/execution/event-buffer.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,49 @@ describe('execution event buffer', () => {
742742
expect(persisted).toContain(LARGE_VALUE_REF_MARKER)
743743
})
744744

745+
/**
746+
* A transient failure while draining the backlog must not cost events, and
747+
* must not let the run be marked terminal. Overwriting the queue would drop
748+
* entries the budget never rejected, and the drain's final chunk would
749+
* otherwise stamp the status before the terminal event is written.
750+
*/
751+
it('retains the backlog and withholds terminal status when the drain fails transiently', async () => {
752+
mockRedis.incrby.mockResolvedValue(100000)
753+
const stamped: string[] = []
754+
let failDrain = true
755+
mockRedis.eval.mockImplementation(async (script: string, ...args: unknown[]) => {
756+
if (!isFlushScript(script)) return [1, 'ok', 0, 0]
757+
const { terminalStatus, zaddArgs } = parseFlushEvalArgs(args)
758+
// Reject any multi-entry batch so the terminal-alone retry path is taken.
759+
if (zaddArgs.length > 2) return [0, 'execution_redis_bytes', 64 * 1024 * 1024]
760+
// The backlog drain hits a transient outage rather than a budget rejection.
761+
if (failDrain) {
762+
failDrain = false
763+
throw new Error('redis unavailable')
764+
}
765+
for (let i = 0; i < zaddArgs.length; i += 2) {
766+
persistedEntries.push(JSON.parse(zaddArgs[i + 1] as string) as ExecutionEventEntry)
767+
}
768+
if (terminalStatus) stamped.push(terminalStatus)
769+
return [1, 1, 0]
770+
})
771+
772+
const payload = 'x'.repeat(1_500_000)
773+
const writer = createExecutionEventWriter('exec-1', {
774+
workspaceId: 'ws-1',
775+
workflowId: 'wf-1',
776+
})
777+
for (let i = 0; i < 3; i++) {
778+
await writer.write(makeEvent(payload)).catch(() => {})
779+
}
780+
await expect(writer.writeTerminal(makeEvent('terminal'), 'complete')).rejects.toThrow()
781+
782+
// The transiently-failed backlog is still queued, so it is not lost.
783+
expect(stamped).toEqual([])
784+
await writer.close().catch(() => {})
785+
expect(persistedEntries.length).toBeGreaterThan(0)
786+
})
787+
745788
it('preserves requested UserFile base64 when buffering terminal events', async () => {
746789
mockRedis.incrby.mockResolvedValue(100)
747790
const base64 = Buffer.from('hello').toString('base64')

apps/sim/lib/execution/event-buffer.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,17 +1094,26 @@ export function createExecutionEventWriter(
10941094
// alone rather than losing the run's final status with them. Gated on a
10951095
// budget rejection specifically: a transient Redis error leaves the batch
10961096
// queued for retry, and clearing it here would turn that into data loss.
1097-
const remaining = pending.filter((pendingEntry) => pendingEntry !== entry)
1098-
// Drain what is queued ahead of the terminal event first. Terminal
1099-
// status is the reader's end-of-run signal: stamping it while lower
1100-
// event ids are still queued strands them behind a stream the reader
1101-
// has already drained and closed.
1102-
if (remaining.length > 0) {
1103-
pending = remaining
1097+
const terminalStatus = pendingTerminalStatus
1098+
pending = pending.filter((pendingEntry) => pendingEntry !== entry)
1099+
if (pending.length > 0) {
1100+
// Drain what is queued ahead of the terminal event first, with the
1101+
// status disarmed: `doFlush` stamps it on whichever chunk empties
1102+
// `pending`, so leaving it armed would mark the run complete before
1103+
// its terminal event is written. Whatever this cannot persist stays
1104+
// queued — it must not be overwritten.
1105+
pendingTerminalStatus = undefined
11041106
await flushPending(false)
1107+
pendingTerminalStatus = terminalStatus
1108+
}
1109+
if (pending.length === 0) {
1110+
// Only publish alone once nothing earlier is still queued. Doing so
1111+
// over a surviving backlog would signal end-of-run to a reader that
1112+
// has not received those events; failing instead lets the caller
1113+
// degrade, which records the status without claiming they arrived.
1114+
pending = [entry]
1115+
ok = await flushPending(false)
11051116
}
1106-
pending = [entry]
1107-
ok = await flushPending(false)
11081117
}
11091118
} catch (error) {
11101119
discardTerminalEntry(entry)

apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,9 +1333,12 @@ export class PauseResumeManager {
13331333
eventType: event.type,
13341334
error: toError(error).message,
13351335
})
1336-
return { eventId: 0, executionId: resumeExecutionId, event }
1336+
return null
13371337
})
1338-
event.eventId = entry.eventId
1338+
// Leave `eventId` unset when the write failed, matching the execute
1339+
// route. Assigning 0 here would be persisted as a reconnect cursor and
1340+
// rewind the client to the start of the run.
1341+
if (entry) event.eventId = entry.eventId
13391342
terminalEventPublished ||= Boolean(terminalStatus)
13401343
}
13411344
sendEvent?.(event)

0 commit comments

Comments
 (0)