Skip to content

fix(execution): stop the event buffer retaining a run-length backlog - #6229

Open
waleedlatif1 wants to merge 1 commit into
stagingfrom
fix/execution-event-buffer-backlog
Open

fix(execution): stop the event buffer retaining a run-length backlog#6229
waleedlatif1 wants to merge 1 commit into
stagingfrom
fix/execution-event-buffer-backlog

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • doFlush's Redis byte-budget branch requeued the rejected batch and rethrew, skipping the MAX_PENDING_EVENTS trim every other failure path applies — so the backlog grew for the rest of the run and each retry re-serialized the whole thing (O(n²)). A wide parallel fan-out could drive unbounded heap growth and stall the event loop.
  • Drop rejected chunks instead of requeueing, and increment consecutiveFlushFailures so the existing backoff actually engages — the old branch threw before the increment, so budget failures retried at the 15ms interval.
  • Split batches above the single-write cap. Two large block outputs coalescing into one batch previously stalled the buffer permanently, since no retry can shrink a batch it keeps whole.
  • Terminal status is writer-scoped rather than call-scoped: a concurrent scheduled flush carries no status of its own and can be the loop that drains the final chunk, which left the terminal event written with no status while writeTerminal reported success. A terminal event whose batch was dropped is retried on its own rather than lost with it.
  • Record terminal stream meta when the terminal event cannot be buffered. The reconnect route already documents this degraded case, but nothing wrote the metadata, so reconnecting readers polled an active stream until the poll deadline and then errored.
  • Remove reserveExecutionRedisBytes/releaseExecutionRedisBytes — zero callers repo-wide. Their TTL invariants are already pinned against the surviving flush script.

Tradeoff

Dropped chunks leave an interior gap in the replay buffer. readExecutionEventsState only signals leading truncation, so a reconnecting client silently misses those events (live SSE delivery is unaffected). Those bytes were never going to persist, so the alternative was retaining them forever. Only signal today is a Dropped execution events… warn.

Type of Change

  • Bug fix

Testing

Unit tests only — each new test verified to fail against the unfixed source before landing. Not yet exercised against a real Redis or a live run; the Lua is mocked in all tests.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 3, 2026 10:58pm

Request Review

@cursor

cursor Bot commented Aug 3, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes core execution streaming and Redis replay durability under budget pressure; reconnect replay may miss dropped events, but live SSE and terminal meta degradation paths are intentional mitigations.

Overview
Fixes execution event buffer behavior when Redis byte budgets reject writes: rejected batches are dropped instead of requeued (which had let pending grow for the whole run and retry at full size), with backoff actually applied on budget failures. Large batches are split so a coalesced chunk above the single-write cap cannot stall the buffer forever.

Terminal stream semantics are tightened: terminal status lives on the writer (pendingTerminalStatus) so a timer-driven flush can still stamp completion; failed terminal publishes clear status so later flushes do not falsely mark the run complete; when the backlog is dropped but the terminal event might fit, it gets a solo flush attempt.

On the workflow execute SSE route, if the replay buffer rejects a terminal event, the live client still gets SSE while setExecutionMeta records terminal status (small HSET) so reconnect readers do not poll an active stream until timeout.

Removes unused reserveExecutionRedisBytes / releaseExecutionRedisBytes (budget enforcement stays in the flush Lua path). Tradeoff: dropped interior replay chunks can leave silent gaps for reconnect replay; live SSE is unaffected.

Reviewed by Cursor Bugbot for commit 7c396f7. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR changes execution-event buffering to bound failed backlogs, split oversized batches, preserve terminal status across concurrent flushes, and record degraded terminal metadata for reconnecting clients.

  • Drops Redis-budget-rejected chunks and applies failure backoff.
  • Splits pending events according to the single-write byte limit.
  • Retries dropped terminal events separately and tracks terminal status at writer scope.
  • Persists terminal metadata when the replay-buffer write fails.
  • Removes unused standalone Redis budget reservation helpers.

Confidence Score: 5/5

The PR appears safe to merge because no eligible blocking follow-up finding remains.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/execution/event-buffer.ts Adds capped chunk flushing, bounded budget-failure handling, writer-scoped terminal state, and isolated terminal retries.
apps/sim/app/api/workflows/[id]/execute/route.ts Persists terminal stream metadata when the terminal replay-buffer write fails after live delivery.
apps/sim/lib/execution/redis-budget.server.ts Removes unused standalone reservation scripts while retaining shared budget limits and key construction.
apps/sim/lib/execution/event-buffer.test.ts Adds coverage for budget rejection, chunk splitting, terminal retries, concurrent flushes, and failure reporting.
apps/sim/app/api/workflows/[id]/execute/route.async.test.ts Verifies degraded terminal metadata is recorded when terminal buffering fails.

Sequence Diagram

sequenceDiagram
  participant Executor
  participant Writer as Event Writer
  participant Redis
  participant Live as Live SSE Client
  participant Replay as Reconnecting Client
  Executor->>Writer: write events
  Writer->>Writer: split pending events into capped chunks
  Writer->>Redis: flush chunk with byte-budget check
  alt Budget accepts chunk
    Redis-->>Writer: persisted
  else Budget rejects chunk
    Redis-->>Writer: rejected
    Writer->>Writer: drop rejected chunk and back off
  end
  Executor->>Writer: write terminal event and status
  alt Terminal write succeeds
    Writer->>Redis: atomically persist event and terminal status
  else Terminal write fails
    Executor->>Live: deliver terminal event live
    Executor->>Redis: persist degraded terminal metadata
  end
  Replay->>Redis: read replay events and metadata
  Redis-->>Replay: events plus authoritative terminal state
Loading

Reviews (4): Last reviewed commit: "fix(execution): stop the event buffer re..." | Re-trigger Greptile

Comment thread apps/sim/lib/execution/event-buffer.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8bed4ae. Configure here.

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1
waleedlatif1 force-pushed the fix/execution-event-buffer-backlog branch from 906c8ea to 412ab18 Compare August 3, 2026 22:55
The Redis byte-budget branch in doFlush requeued the rejected batch and
rethrew, skipping the MAX_PENDING_EVENTS trim every other failure path
applies. The backlog then grew for the rest of the run and each retry
re-serialized it, so a wide parallel fan-out could drive unbounded heap
growth and stall the event loop.

Drop rejected chunks instead of requeueing, pace retries through the
existing backoff, and split batches that exceed the single-write cap so
an oversized batch can make progress instead of stalling forever.
Terminal status is now writer-scoped, since a concurrent scheduled flush
can be the loop that drains the final chunk, and a terminal event whose
batch was dropped is retried on its own rather than lost with it.

Record terminal stream meta when the terminal event cannot be buffered,
so reconnecting readers stop polling an active stream until their
deadline. Drop the unused reserve/release budget helpers.
@waleedlatif1
waleedlatif1 force-pushed the fix/execution-event-buffer-backlog branch from 412ab18 to 7c396f7 Compare August 3, 2026 22:58
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 7c396f7. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant