feat: async workflows #306

thantos · 2023-03-03T19:42:34Z

Complete re-implementation of the workflow re-entry interpreter.

Highlights:

Works with async/Promises, does not require a transformer
Abstracts the eventuals into a consistent contract (decoupled from the interepter)
Interpreter has been renamed to WorkflowExecutor, implemented as a class
Starting support for long running, in memory workflows. Bootstrap with history and then keep sending in new events until complete.

TODO:

More testing
- Concurrency
- Dangling promises
- Runtime tests
Cancellation - promises currently do not cancel
Clean up and straighten out terms - disgusting right now
Look into turning logging and date hooks into context and plug-ins

packages/@eventual/core-runtime/src/workflow-executor.ts

sam-goodwin · 2023-03-04T05:47:49Z

packages/@eventual/core-runtime/src/workflow-executor.ts

+    return new Promise(async (resolve) => {
+      // start context with execution hook


lol new Promise(async(resolve) => is an interesting pattern. Was there a reason to not do (async () => {})()

Because I am using the resolve method to complete from other contexts.

this.started = { // TODO, also cancel? resolve: (result) => { const newCommands = this.commandsToEmit; this.commandsToEmit = []; resolve({ commands: newCommands, result }); }, };

When something needs to complete the workflow with a forced value like a value, the this.forceComplete method is called which calls the resolve on the top promise.

So callers of start have a reference to the run being completed.

packages/@eventual/core-runtime/src/workflow-executor.ts

sam-goodwin · 2023-03-04T05:51:50Z

packages/@eventual/core-runtime/src/workflow-executor.ts

+  public async continue(
+    ...history: HistoryEvent[]
+  ): Promise<WorkflowResult<Output>> {


nice! Looking FWD to this. Will be very useful in an environment like CF.

Add some docs?

Also needs to be tested, don't think it works in the current state.

packages/@eventual/core-runtime/src/workflow-executor.ts

sam-goodwin · 2023-03-04T05:59:02Z

packages/@eventual/core-runtime/src/workflow-executor.ts

+      await new Promise((resolve) => {
+        setTimeout(() => {
+          this.tryCommitResultEvent(event);
+          resolve(undefined);
+        });
+      });
+      // TODO: do we need to use setTimeout here to go to the end of the event loop?


Good question. It comes down to how tryCommitResultEvent behaves. I'm leaning towards not needing it here. Calling tryCommitResultEvent will call resolve and reject which I think should exhaustively progress the promise chains before returning.

Looks like yes, without this, the next events will be added out of order.

const wf = workflow(async () => { const result = await createExpectSignalCall( "MySignal", createAwaitTimerCall(Schedule.duration(100 * 1000, "seconds")) ); return result ?? "done"; });

In this case, without set timeout, the timer command it not in the output array when the workflow resolves. Likely because the events are all emitted before the calls are able to register their eventuals.

And the promise allows this to be iterative instead of recursive.

I remember a fun bug in xstate where they used recursion and I hit some depth limits.

sam-goodwin · 2023-03-04T06:00:33Z

packages/@eventual/core-runtime/src/workflow-executor.ts

+          eventual.resolve(result.value);
+        } else if (isFailed(result)) {
+          eventual.reject(result.error);


I assume this will synchronously step in to and begin completing the promise chain.

Perhaps this means you don't need the setTimeout in drainHistoryEvents because this code will only return after all of the promises that can resume have resumed? Unless they do IO or something dumb, there shouldn't be anything outstanding on the event loop?

I actually ran into issues where I needed to add the setTimeout, the while look would drain all events before the promise resolves would return to the caller.

So I think the local syncronous thread is preferred over the .then or continuing from await.

That said, I am going to try to roll things back and see what breaks.

thantos · 2023-03-07T03:41:55Z

apps/tests/aws-runtime/scripts/test-cli

+npx eventual start workflow sleepy
+
+npx eventual start workflow sleepy -f
+
+npx eventual list executions

-npx eventual list executions --workflow parallel --json > .eventual/out.json
+npx eventual list executions --workflow sleepy --json > .eventual/out.json


This test was not using an execution that was started by the same script, which was confusing and could be faulty.

packages/@eventual/compiler/src/eventual-bundle.ts

packages/@eventual/core-runtime/src/eventual-factory.ts

sam-goodwin · 2023-03-07T06:20:12Z

packages/@eventual/core-runtime/src/eventual-factory.ts

+    return {
+      triggers: [
+        Trigger.workflowEvent(
+          WorkflowEventType.ChildWorkflowSucceeded,
+          (event) => Result.resolved(event.result)
+        ),
+        Trigger.workflowEvent(WorkflowEventType.ChildWorkflowFailed, (event) =>
+          Result.failed(new EventualError(event.error, event.message))
+        ),
+        call.timeout
+          ? Trigger.promise(call.timeout, () =>
+              Result.failed("Child Workflow Timed Out")
+            )
+          : undefined,
+      ],
+      generateCommands(seq) {
+        return {
+          kind: CommandType.StartWorkflow,
+          seq,
+          name: call.name,
+          input: call.input,
+          opts: call.opts,
+        };
+      },
+    };


I don't know what's going on here. Seems like an over abstraction? It seems to be associating event types with callbacks? What problem was this solving?

This pulls out all of the call specific behavior out of the interpreter.

Triggers - when the Eventual can be resolved and what to do (callback) - events, signal, every change (ex: condition), and another promise/eventual (ex: activity.timeout)

GenerateCommands - Replaces the callToCommand method

The triggers also allow us to create lookup tables for eventuals (via seq), events, and signals instead of searching for things to change.

Why did it need to be separated out?

The behavior of each call was spread around the interpreter.

Some things ran all of the time, some emitted commands, some didn't, some were waiting on other promises, etc.

Now the behavior is a well defined contract.

Should make adding new ones trivial as well. Potentially pluggable.

packages/@eventual/core-runtime/src/workflow-executor.ts

sam-goodwin · 2023-03-07T06:24:55Z

packages/@eventual/core-runtime/src/workflow-executor.ts

+  /**
+   * Starts an execution.
+   *
+   * The execution will run until completion or until the events are exhausted.


Curious what happens if it returns while things are still running in the background?

Good question, resolve on the top level promise can only be called once, so that works well.

The only thing I would be concerned about is the commandToEmit array which is stateful, the array get cleared on completion.

If you are asking about a workflow that has ongoing eventuals like dangling promises or signal handlers, I don't think we allow them right now... or we are inconsistent. I... will write more tests. I think at one point we said we want workflows to execute past completion, so maybe that is wrong right now. Our tests didn't cover that case.

It did used to work and we discussed it before. I guess we didn't add a test. It would be important for signal handlers and also for things like queries if we ever add them

Updated and tested, succeeded workflows will continue to accept events and emit events. Failed will not.

sam-goodwin · 2023-03-07T08:49:00Z

packages/@eventual/core-runtime/src/date-hook.ts

-        super(...args);
+export function hookDate() {
+  if (!isDateHooked()) {
+    globalThis.Date = class extends Date {


Do you need to be able to restore it?

There is a restoreDate method.

…less/eventual into sussman/async_workflows

sam-goodwin

Your best work yet. Amazing stuff. Let's go. Had some comments but I think it's ready.

sam-goodwin · 2023-03-09T04:33:18Z

packages/@eventual/core-runtime/src/eventual-factory.ts

+  if (isActivityCall(call)) {
+    return {
+      triggers: [
+        Trigger.workflowEvent(WorkflowEventType.ActivitySucceeded, (event) =>


does this mean "onWorkflowEvent"? Would that be a better name? Or just onEvent?

sam-goodwin · 2023-03-09T04:34:21Z

packages/@eventual/core-runtime/src/workflow-executor.ts

+  seq: number;
+}
+
+export function createEventualPromise<R>(


document why this is needed

packages/@eventual/core-runtime/src/workflow-executor.ts

sam-goodwin · 2023-03-09T04:57:13Z

packages/@eventual/core-runtime/src/workflow-executor.ts

+  commands: WorkflowCommand[];
+}
+
+export type Trigger<OwnRes> =


What does OwnRes mean?

Own result as opposed to the result of the trigger. Will make this more clear.

packages/@eventual/core-runtime/src/workflow-executor.ts

thantos · 2023-03-09T16:31:38Z

Your best work yet. Amazing stuff. Let's go. Had some comments but I think it's ready.

takes a bow Great feedback, updated, will merge once the tests pass.

thantos added 4 commits March 3, 2023 04:41

promise hook and tests

2e66902

feat: async workflow executor

f22dd6c

fix...

057b074

tests pass

8484f38

sam-goodwin reviewed Mar 4, 2023

View reviewed changes

thantos added 10 commits March 4, 2023 18:50

feedback

4f35d15

simplify test env, fix bugs, remove async transformer

a00fd19

more test refactoring

ce4a7d1

fix replay bug

8da98d8

signal handler access to workflow hook

c4ea0ea

move commands to core runtime

95116f2

simplify await timer calls

aec39b7

refactor executor

44bcb65

old spelling error

a0f7b94

test continue

1ac48bd

thantos marked this pull request as ready for review March 6, 2023 23:01

thantos added 2 commits March 6, 2023 17:19

fix some tests

94ef5f7

fix tests

724e7ac

thantos requested a review from sam-goodwin March 7, 2023 03:39

thantos commented Mar 7, 2023

View reviewed changes

packages/@eventual/compiler/src/eventual-bundle.ts Outdated Show resolved Hide resolved

thantos added 3 commits March 6, 2023 22:43

cleanup

31fd5fa

date hook with async local store

fe4538d

docs

58f34bc

sam-goodwin reviewed Mar 7, 2023

View reviewed changes

thantos added 5 commits March 7, 2023 11:06

support events after workflow success

3141700

clean up

8755120

fix date bug

169e295

revert als

cf3069d

Merge branch 'sussman/async_workflows' of https://github.com/function…

7ef962f

…less/eventual into sussman/async_workflows

thantos added 5 commits March 7, 2023 20:09

Merge branch 'main' into sussman/async_workflows

9f81bf7

workflow failures

1bce98b

reenable the chaos ext

b262d57

speed and clean up

fb25b0f

testing then, catch, finally

944dd90

thantos requested a review from sam-goodwin March 9, 2023 04:29

sam-goodwin approved these changes Mar 9, 2023

View reviewed changes

feedback

bb6a982

thantos merged commit f048ae9 into main Mar 9, 2023

thantos deleted the sussman/async_workflows branch March 9, 2023 17:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: async workflows #306

feat: async workflows #306

thantos commented Mar 3, 2023 •

edited

Loading

sam-goodwin Mar 4, 2023

thantos Mar 4, 2023

sam-goodwin Mar 4, 2023

thantos Mar 4, 2023

sam-goodwin Mar 4, 2023

thantos Mar 6, 2023

thantos Mar 6, 2023

sam-goodwin Mar 4, 2023

thantos Mar 5, 2023

thantos Mar 7, 2023

sam-goodwin Mar 7, 2023

thantos Mar 7, 2023

sam-goodwin Mar 7, 2023

thantos Mar 7, 2023

sam-goodwin Mar 7, 2023

thantos Mar 7, 2023

sam-goodwin Mar 7, 2023

thantos Mar 7, 2023

sam-goodwin Mar 7, 2023

thantos Mar 7, 2023

sam-goodwin left a comment

sam-goodwin Mar 9, 2023

sam-goodwin Mar 9, 2023

sam-goodwin Mar 9, 2023

thantos Mar 9, 2023

thantos commented Mar 9, 2023

		return new Promise(async (resolve) => {
		// start context with execution hook

feat: async workflows #306

feat: async workflows #306

Conversation

thantos commented Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sam-goodwin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thantos commented Mar 9, 2023

thantos commented Mar 3, 2023 •

edited

Loading