-
Notifications
You must be signed in to change notification settings - Fork 307
feat(core): experimental in-process VM continuation for inline replay #2966
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@workflow/core': minor | ||
| --- | ||
|
|
||
| Add in-process VM continuation (on by default; kill switch `WORKFLOW_VM_CONTINUATION=0`). The inline replay loop now keeps a suspended workflow VM alive and feeds newly-appended events into it for step-only suspensions, instead of rebuilding the `vm.Context` and replaying the event log from scratch each pass. Falls back to a full replay on any prefix divergence or non-step suspension, so the kill switch restores the previous behavior exactly. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -293,6 +293,34 @@ export function isInlineOwnershipEnabled(): boolean { | |
| return !(raw === '0' || raw.toLowerCase() === 'false'); | ||
| } | ||
|
|
||
| /** | ||
| * Whether in-process VM continuation is enabled (default OFF — experimental). | ||
| * | ||
| * When on, the inline replay loop keeps a suspended workflow VM alive across | ||
| * iterations instead of rebuilding the `vm.Context` and replaying the whole | ||
| * event log from event 1 on every pass. After an inline step's terminal event | ||
| * is appended, the loop feeds the newly-appended events into the SAME live VM | ||
| * (resolving the pending step promise) so the workflow body advances to its | ||
| * next suspension point — skipping the from-scratch replay entirely. | ||
| * | ||
| * This is sound ONLY within a single invocation's inline loop: the same process | ||
| * holds the VM and the same handler owns the appended writes, so the durable | ||
| * event log stays authoritative. It is scoped further to *simple, step-only* | ||
| * suspensions (no hooks/waits/attribute writes); any other suspension, or any | ||
| * divergence between the live VM's consumed prefix and the authoritative log, | ||
| * makes the continuation bail out and the loop falls back to a full replay in | ||
| * a fresh VM. Worst case is therefore identical to the from-scratch replay path. | ||
| * | ||
| * Default **ON** so CI and benchmarks exercise (and measure) the continuation | ||
| * path. `WORKFLOW_VM_CONTINUATION=0` (or `false`) is the kill switch that | ||
| * reverts to rebuilding the VM and replaying from scratch on every pass. | ||
| */ | ||
| export function isVmContinuationEnabled(): boolean { | ||
| const raw = process.env.WORKFLOW_VM_CONTINUATION; | ||
| if (raw === undefined || raw === '') return true; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Default-on posture: this flag is read at runtime in production too (as the PR notes), and combined with the open quiescence gap above, the draft/POC status, and no execution-mode telemetry, that is a lot to enable by default. I'd flip the default to off here until the sandbox is quiesced (or this converges onto #2990's hardened sandbox), and keep default-on only in the CI/benchmark lanes via an explicit env in those workflows. As written, the first production workflow that races a host-timed primitive against a step gets a silently corrupted run with no signal -- the worst failure mode for a durability product. |
||
| return !(raw === '0' || raw.toLowerCase() === 'false'); | ||
| } | ||
|
|
||
| /** | ||
| * Default inline-ownership lease, in seconds: how long after a step's latest | ||
| * (stamped) `step_started` a non-owner invocation assumes the owning | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The prefix guard is necessary but not sufficient. It validates that the already-consumed prefix (
0..eventIndex) is byte-stable, which catches a rewritten log -- but it cannot detect a VM that took a different future branch after the cursor because it advanced on host timing during the gap (see theisContinuationEligiblecomment). The consumed prefix still matches; the divergence is in the events the retained VM is about to produce. #2990 adds a boundary-stability check (isSameSuspensionBoundary, demoting to replay if the VM re-suspends on anything other than the same steps) precisely for this; this PR trusts a non-quiesced advance silently. At minimum, assert the VM re-suspends on the same step set before adopting the appended events.Minor, same method: dropping
readonlyand doingthis.events = eventsmakes the consumer alias the caller's array rather than own a copy (#2990 copies in the ctor andappend()s only the delta). It works today because the loop happens to pass the same array, but it couples the consumer's internal invariant to the caller's array handling; copy-and-append is less fragile.