Skip to content

Commit f210a6e

Browse files
authored
fix(execution): offload buffered event values under budget pressure (#6231)
* fix(execution): offload buffered event values under budget pressure An execution buffers EVENT_LIMIT events inside a per-execution byte budget, so a full ring only fits if events average under budget/EVENT_LIMIT. Values were only offloaded to object storage at the shared 8 MiB cap, far above that, so a run emitting large block outputs exhausted its budget within a few dozen events and stayed pinned at its ceiling for the rest of its life. Applying that ceiling to every run would be worse than the problem: the SSE stream carries the compacted event and the terminal renders a ref only as a preview, so ordinary block outputs would stop being readable live, and every value would cost an object-storage write on the hot path. Engage the tight ceiling only once a run has actually buffered past half its budget. A short run keeps full-fidelity output and pays nothing; a runaway one stops accumulating. Both bounds derive from the existing budget rather than being asserted, and preserved UserFile base64 is exempt — it is an explicit request for inline delivery, already bounded by its own cap and the strip-and-recompact fallback. Also stop a failed resume-path buffer write from failing the run: it was awaited bare, so the rejection propagated into the executor callback and failed work that had already completed. The buffer only backs reconnect replay, so degrade to live-only delivery the way the execute route does. * fix(execution): measure pressure at write time and keep terminal status last Pressure was read from bytes counted once a flush succeeded, but a burst is compacted long before the scheduled flush runs — so the very batch that exhausts the budget went through at the loose ceiling and was dropped instead of offloaded. Count bytes as each event is compacted. Separately, the terminal-alone retry stamped terminal status while entries queued ahead of it were still unwritten. Terminal status is the reader's end-of-run signal: a reconnecting client drains what is in Redis and closes, so those entries were stranded behind a stream it had already finished with. Drain the backlog first, then publish the terminal event. * 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. * fix(execution): keep an event in the buffer when a pressure offload fails Durable compaction runs before an event is queued, so a storage or metadata failure dropped it from replay entirely — a reconnecting client would never see it, even though the live path carried on. Offloading under pressure is only an optimization that keeps a heavy run from exhausting its budget, so when the value cannot be persisted, fall back to buffering it inline: exactly what the run would have done before pressure engaged.
1 parent 41592df commit f210a6e

3 files changed

Lines changed: 287 additions & 16 deletions

File tree

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

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { redisConfigMockFns, resetRedisConfigMock } from '@sim/testing'
55
import { sleep } from '@sim/utils/helpers'
66
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
77
import type { ExecutionEventEntry } from '@/lib/execution/event-buffer'
8+
import { LARGE_VALUE_REF_MARKER } from '@/lib/execution/payloads/large-value-ref'
89
import type { ExecutionEvent } from '@/lib/workflows/executor/execution-events'
910

1011
const { mockRedis, persistedEntries } = vi.hoisted(() => {
@@ -622,6 +623,188 @@ describe('execution event buffer', () => {
622623
await expect(writer.flush()).resolves.toBeUndefined()
623624
})
624625

626+
/**
627+
* A short run must keep full-fidelity output: the SSE stream carries the
628+
* compacted event, and the terminal renders a ref only as a preview, so
629+
* offloading ordinary block outputs would make them unreadable live.
630+
*/
631+
it('keeps values inline while the execution is below the offload pressure mark', async () => {
632+
mockRedis.incrby.mockResolvedValue(100)
633+
const payload = 'x'.repeat(512 * 1024)
634+
635+
const writer = createExecutionEventWriter('exec-1', {
636+
workspaceId: 'ws-1',
637+
workflowId: 'wf-1',
638+
})
639+
await writer.write(makeEvent(payload))
640+
await writer.flush()
641+
642+
const persisted = JSON.stringify(persistedEntries[0])
643+
expect(persisted).toContain(payload)
644+
expect(persisted).not.toContain(LARGE_VALUE_REF_MARKER)
645+
})
646+
647+
/**
648+
* Once a run has buffered its way into the danger zone the tight ceiling
649+
* engages, so it stops accumulating against its budget instead of pinning
650+
* itself at the ceiling for the rest of its life.
651+
*/
652+
it('offloads values once the execution crosses the offload pressure mark', async () => {
653+
mockRedis.incrby.mockResolvedValue(100000)
654+
const payload = 'x'.repeat(2 * 1024 * 1024)
655+
656+
const writer = createExecutionEventWriter('exec-1', {
657+
workspaceId: 'ws-1',
658+
workflowId: 'wf-1',
659+
})
660+
// Push past half the per-execution budget so the next write is under pressure.
661+
for (let i = 0; i < 17; i++) {
662+
await writer.write(makeEvent(payload))
663+
await writer.flush()
664+
}
665+
persistedEntries.length = 0
666+
await writer.write(makeEvent(payload))
667+
await writer.flush()
668+
669+
const persisted = JSON.stringify(persistedEntries[0])
670+
expect(persisted).toContain(LARGE_VALUE_REF_MARKER)
671+
expect(persisted).not.toContain(payload)
672+
})
673+
674+
/**
675+
* Terminal status is the reader's end-of-run signal: once it lands, a
676+
* reconnecting client drains what is in Redis and closes. Stamping it while
677+
* lower event ids are still queued strands those events behind a stream the
678+
* reader has already finished with.
679+
*
680+
* Needs a backlog past the single-write cap so chunking leaves a remainder
681+
* behind the terminal entry — the only shape where that ordering can invert.
682+
*/
683+
it('does not stamp terminal status while earlier events are still queued', async () => {
684+
mockRedis.incrby.mockResolvedValue(100000)
685+
const idsAtStamp: number[] = []
686+
mockRedis.eval.mockImplementation(async (script: string, ...args: unknown[]) => {
687+
if (!isFlushScript(script)) return [1, 'ok', 0, 0]
688+
const { terminalStatus, zaddArgs } = parseFlushEvalArgs(args)
689+
// Reject any multi-entry batch, forcing the terminal-alone retry path.
690+
if (zaddArgs.length > 2) return [0, 'execution_redis_bytes', 64 * 1024 * 1024]
691+
for (let i = 0; i < zaddArgs.length; i += 2) {
692+
persistedEntries.push(JSON.parse(zaddArgs[i + 1] as string) as ExecutionEventEntry)
693+
}
694+
if (terminalStatus && idsAtStamp.length === 0) {
695+
idsAtStamp.push(...persistedEntries.map((e) => e.eventId))
696+
}
697+
return [1, 1, 0]
698+
})
699+
700+
// ~3MB per event, so three of them exceed the 8MiB single-write cap and the
701+
// chunk boundary leaves a remainder queued behind the terminal entry.
702+
const payload = 'x'.repeat(1_500_000)
703+
const writer = createExecutionEventWriter('exec-1', {
704+
workspaceId: 'ws-1',
705+
workflowId: 'wf-1',
706+
})
707+
for (let i = 0; i < 3; i++) {
708+
await writer.write(makeEvent(payload)).catch(() => {})
709+
}
710+
await writer.writeTerminal(makeEvent('terminal'), 'complete').catch(() => {})
711+
await writer.close().catch(() => {})
712+
713+
const terminalId = Math.max(...persistedEntries.map((e) => e.eventId))
714+
const strandedAtStamp = persistedEntries
715+
.map((e) => e.eventId)
716+
.filter((id) => id < terminalId && !idsAtStamp.includes(id))
717+
expect(strandedAtStamp).toEqual([])
718+
})
719+
720+
/**
721+
* Pressure has to be measured as events are produced, not once a flush
722+
* succeeds. A burst is compacted long before the scheduled flush runs, so
723+
* flush-time accounting would let the very batch that exhausts the budget
724+
* through at the loose ceiling and drop it instead of offloading it.
725+
*/
726+
it('engages pressure within a burst that has not flushed yet', async () => {
727+
mockRedis.incrby.mockResolvedValue(100000)
728+
const payload = 'x'.repeat(2 * 1024 * 1024)
729+
730+
const writer = createExecutionEventWriter('exec-1', {
731+
workspaceId: 'ws-1',
732+
workflowId: 'wf-1',
733+
})
734+
// No flush between writes: everything stays pending while the burst builds.
735+
for (let i = 0; i < 20; i++) {
736+
await writer.write(makeEvent(payload)).catch(() => {})
737+
}
738+
await writer.flush().catch(() => {})
739+
740+
// The later events in the burst must have been offloaded, not left inline.
741+
const persisted = JSON.stringify(persistedEntries)
742+
expect(persisted).toContain(LARGE_VALUE_REF_MARKER)
743+
})
744+
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+
788+
/**
789+
* Offloading under pressure is an optimization. If the value cannot be
790+
* persisted durably, the event must still reach the replay buffer inline —
791+
* dropping it would leave a reconnecting client permanently missing it.
792+
*/
793+
it('buffers the event inline when a pressure offload cannot be persisted', async () => {
794+
mockRedis.incrby.mockResolvedValue(100000)
795+
const payload = 'x'.repeat(2 * 1024 * 1024)
796+
797+
// No workspace/workflow ids, so durable persistence of an offloaded value
798+
// fails the way a storage outage would.
799+
const writer = createExecutionEventWriter('exec-1')
800+
for (let i = 0; i < 20; i++) {
801+
await writer.write(makeEvent(payload)).catch(() => {})
802+
}
803+
await writer.flush().catch(() => {})
804+
805+
expect(persistedEntries).toHaveLength(20)
806+
})
807+
625808
it('preserves requested UserFile base64 when buffering terminal events', async () => {
626809
mockRedis.incrby.mockResolvedValue(100)
627810
const base64 = Buffer.from('hello').toString('base64')

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

Lines changed: 88 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,21 @@ const FLUSH_INTERVAL_MS = 15
2727
const FLUSH_MAX_RETRY_INTERVAL_MS = 1000
2828
const FLUSH_MAX_BATCH = 200
2929
const MAX_PENDING_EVENTS = 1000
30+
/**
31+
* Bytes a single execution may buffer before its events start offloading
32+
* aggressively, and the per-value threshold applied once it does.
33+
*
34+
* The buffer holds `EVENT_LIMIT` events inside the per-execution byte budget,
35+
* so a full ring only fits if events average under budget/EVENT_LIMIT. Applying
36+
* that ceiling to every run would offload ordinary block outputs into refs the
37+
* terminal cannot display — the SSE stream carries the compacted event, and a
38+
* ref renders only as a preview. Instead the tight ceiling engages only once a
39+
* run has actually buffered its way into the danger zone, so a short run keeps
40+
* full-fidelity output and a runaway one stops accumulating.
41+
*/
42+
const EXECUTION_EVENT_OFFLOAD_PRESSURE_BYTES = getExecutionRedisBudgetLimits().maxExecutionBytes / 2
43+
const EXECUTION_EVENT_PRESSURE_VALUE_BYTES =
44+
getExecutionRedisBudgetLimits().maxExecutionBytes / EVENT_LIMIT
3045
const ACTIVE_META_ATTEMPTS = 3
3146
const FINALIZE_FLUSH_ATTEMPTS = 2
3247
const FLUSH_EVENTS_SCRIPT = `
@@ -282,6 +297,8 @@ export interface ExecutionEventWriter {
282297
export interface ExecutionEventWriterContext extends LargeValueStoreContext {
283298
requireDurablePayloads?: boolean
284299
preserveUserFileBase64?: boolean
300+
/** Offload ceiling for individual values; defaults to the shared large-value cap. */
301+
valueThresholdBytes?: number
285302
}
286303

287304
async function compactEventForBuffer(
@@ -297,6 +314,7 @@ async function compactEventForBuffer(
297314
executionId: context.executionId ?? event.executionId,
298315
requireDurable: context.requireDurablePayloads,
299316
preserveRoot: true,
317+
thresholdBytes: context.valueThresholdBytes,
300318
}
301319

302320
let compactedData = await compactExecutionPayload(event.data, {
@@ -746,6 +764,27 @@ export function createExecutionEventWriter(
746764
let maxReservedId = 0
747765
let flushTimer: ReturnType<typeof setTimeout> | null = null
748766
let consecutiveFlushFailures = 0
767+
/**
768+
* Bytes this execution has produced, counted as each event is compacted
769+
* rather than once a flush succeeds. A burst can be compacted long before the
770+
* scheduled flush runs, so flush-time accounting would let the very batch that
771+
* exhausts the budget through at the loose ceiling. Counted gross of
772+
* ring-buffer pruning too, so the mark is reached early — erring toward
773+
* offloading sooner is the safe direction.
774+
*/
775+
let bufferedBytes = 0
776+
777+
/**
778+
* Preserved base64 is an explicit request for inline delivery and is already
779+
* bounded by its own cap and the strip-and-recompact fallback, so pressure
780+
* never rewrites it into a ref the caller cannot read.
781+
*/
782+
const getValueThresholdBytes = () => {
783+
if (context.preserveUserFileBase64) return undefined
784+
return bufferedBytes >= EXECUTION_EVENT_OFFLOAD_PRESSURE_BYTES
785+
? EXECUTION_EVENT_PRESSURE_VALUE_BYTES
786+
: undefined
787+
}
749788

750789
const getFlushDelayMs = () => {
751790
if (consecutiveFlushFailures === 0) return FLUSH_INTERVAL_MS
@@ -983,17 +1022,39 @@ export function createExecutionEventWriter(
9831022
}
9841023
}
9851024

1025+
/**
1026+
* Compact an event for the buffer, degrading if pressure offloading fails.
1027+
*
1028+
* Offloading under pressure is an optimization: it keeps a heavy run from
1029+
* exhausting its budget. When durable storage rejects the write, losing the
1030+
* event from replay entirely is a worse outcome than carrying it inline, so
1031+
* fall back to the shared cap — exactly what the run would have done before
1032+
* pressure engaged.
1033+
*/
1034+
const compactForBuffer = async (event: ExecutionEvent) => {
1035+
const valueThresholdBytes = getValueThresholdBytes()
1036+
const options = { ...context, executionId, requireDurablePayloads: true }
1037+
if (valueThresholdBytes === undefined) return compactEventForBuffer(event, options)
1038+
try {
1039+
return await compactEventForBuffer(event, { ...options, valueThresholdBytes })
1040+
} catch (error) {
1041+
logger.warn('Pressure offload failed; buffering the event inline instead', {
1042+
executionId,
1043+
eventType: event.type,
1044+
error: toError(error).message,
1045+
})
1046+
return compactEventForBuffer(event, options)
1047+
}
1048+
}
1049+
9861050
const writeCore = async (event: ExecutionEvent): Promise<ExecutionEventEntry> => {
9871051
if (nextEventId === 0 || nextEventId > maxReservedId) {
9881052
await reserveIds(1)
9891053
}
9901054
const eventId = nextEventId++
991-
const compactEvent = await compactEventForBuffer(event, {
992-
...context,
993-
executionId,
994-
requireDurablePayloads: true,
995-
})
1055+
const compactEvent = await compactForBuffer(event)
9961056
const entry: ExecutionEventEntry = { eventId, executionId, event: compactEvent }
1057+
bufferedBytes += getJsonSize(entry) ?? 0
9971058
pending.push(entry)
9981059
if (pending.length >= FLUSH_MAX_BATCH) {
9991060
await flushPending()
@@ -1034,12 +1095,9 @@ export function createExecutionEventWriter(
10341095
await reserveIds(1)
10351096
}
10361097
const eventId = nextEventId++
1037-
const compactEvent = await compactEventForBuffer(event, {
1038-
...context,
1039-
executionId,
1040-
requireDurablePayloads: true,
1041-
})
1098+
const compactEvent = await compactForBuffer(event)
10421099
const entry: ExecutionEventEntry = { eventId, executionId, event: compactEvent }
1100+
bufferedBytes += getJsonSize(entry) ?? 0
10431101
pending.push(entry)
10441102
let ok = false
10451103
try {
@@ -1051,10 +1109,26 @@ export function createExecutionEventWriter(
10511109
// alone rather than losing the run's final status with them. Gated on a
10521110
// budget rejection specifically: a transient Redis error leaves the batch
10531111
// queued for retry, and clearing it here would turn that into data loss.
1054-
const remaining = pending.filter((pendingEntry) => pendingEntry !== entry)
1055-
pending = [entry]
1056-
ok = await flushPending(false)
1057-
pending = pending.concat(remaining)
1112+
const terminalStatus = pendingTerminalStatus
1113+
pending = pending.filter((pendingEntry) => pendingEntry !== entry)
1114+
if (pending.length > 0) {
1115+
// Drain what is queued ahead of the terminal event first, with the
1116+
// status disarmed: `doFlush` stamps it on whichever chunk empties
1117+
// `pending`, so leaving it armed would mark the run complete before
1118+
// its terminal event is written. Whatever this cannot persist stays
1119+
// queued — it must not be overwritten.
1120+
pendingTerminalStatus = undefined
1121+
await flushPending(false)
1122+
pendingTerminalStatus = terminalStatus
1123+
}
1124+
if (pending.length === 0) {
1125+
// Only publish alone once nothing earlier is still queued. Doing so
1126+
// over a surviving backlog would signal end-of-run to a reader that
1127+
// has not received those events; failing instead lets the caller
1128+
// degrade, which records the status without claiming they arrived.
1129+
pending = [entry]
1130+
ok = await flushPending(false)
1131+
}
10581132
}
10591133
} catch (error) {
10601134
discardTerminalEntry(entry)

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,8 +1323,22 @@ export class PauseResumeManager {
13231323
await degradeTerminalPublish(terminalStatus, error)
13241324
return { eventId: 0, executionId: resumeExecutionId, event }
13251325
})
1326-
: await eventWriter.write(event)
1327-
event.eventId = entry.eventId
1326+
: await eventWriter.write(event).catch((error) => {
1327+
// The buffer only backs reconnect replay; the live stream is the
1328+
// primary delivery path. Awaiting this bare let a failed write
1329+
// propagate into the executor callback and fail work that had
1330+
// already run, so degrade the same way the execute route does.
1331+
logger.warn('Resume event buffer write failed; delivering live only', {
1332+
resumeExecutionId,
1333+
eventType: event.type,
1334+
error: toError(error).message,
1335+
})
1336+
return null
1337+
})
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
13281342
terminalEventPublished ||= Boolean(terminalStatus)
13291343
}
13301344
sendEvent?.(event)

0 commit comments

Comments
 (0)