Skip to content

Conversation

@jeffschoner-stripe
Copy link
Contributor

This contains several follow ups to #111 around improving correctness, performance, and API ergonomics of the use of wait_for with a block condition. This is equivalent to the await method in Go and Java SDKs (Go docs).

Split wait_for into dedicated methods

The wait_for method on workflow context started as a way to wait on a single future to complete. It was rarely called directly because activity or timer futures have their own .wait methods that called through to this method. With the introduction of a conditional block to wait_for, this signature became much more complicated, taking both a splat of futures and a conditional block. Note this is API breaking because the wait_for method is going away. This method should rarely be called for activities and timers since futures already have an equivalent wait method. The use of wait_for with a condition block or with wait-for-any semantics will need to be rewritten using the new methods.

With this split, it is now two methods:

  • wait_for_any which takes a splat of futures, and blocks workflow progression until at least one future is completed. This is similar to the already existing wait_for_all.
  • wait_until which takes a block, and blocks until that block evaluates to true. Complex combinations of futures and workflow state can be combined into a single conditional block. For example, waiting on a timer firing or a signal being received can be done with something like,
signal = nil
workflow.on_signal |name, input| do
  signal = input
end

timer = workflow.start_timer(60)
workflow.wait_until { !signal.nil? || timer.finished? }
# examine state here

Deterministically order wildcard dispatch handlers

When wait_until is called, a dispatch function is added to resume the fiber once the condition has been satisfied. Because this condition could change due to any workflow progress, it must be evaluated on every dispatch. Before this change, these were always evaluated after any targeted dispatch callbacks (such as from a specific activity or timer). This can cause non-determinism in certain somewhat corner cases. These callbacks should always be called in the same order. This is achieved by associating an autoincrementing, unique ID with each handler. When the list of handlers is concatenated and filtered, it is now sorted by these IDs, guaranteeing order across replays.

Remove dispatch handlers once they're no longer needed

In activities with long histories, the number of dispatch handlers can get quite large. Particularly for workflows that call wait_until many times (such as in a loop), the number of these can get very large, and they must be invoked on every dispatch. This can cause performance problems that result in timed out workflow tasks. The fix in this change removes these once they're no longer needed. The unique autoincrementing ID mentioned in the previous section is used to identify and remove callbacks once they are no longer needed, dramatically improving performance for workflows with long histories.

expect(handler_1).to have_received(:call).ordered
expect(handler_4).to have_received(:call).ordered
expect(handler_6).to have_received(:call).ordered
expect(handler_7).to have_received(:call).ordered
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before this change, the sequence here would have been 1, 4, 7, 6 instead because targeted handlers were invoked before wildcard handlers

@jeffschoner-stripe
Copy link
Contributor Author

I'm going to break this up into smaller PRs

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant