Improve determinism explanation #3614
Conversation
| - Ending the Workflow Execution in any way (completing, failing, cancelling, or continuing as new) | ||
| - Patched or GetVersion calls for versioning (though they may be added or removed according to the [patching](#patching) rules) | ||
| - Upserting Workflow search attributes | ||
| - Upserting Workflow memos |
There was a problem hiding this comment.
nit: can include SideEffect and MutableSideEffect here too
|
|
||
| More formally, the use of certain Workflow APIs in the function is what generates | ||
| [Commands](/workflow-execution#command). Commands tell the Temporal Service which Events to create | ||
| and add to the Workflow Execution's [Event History](/workflow-execution/event#event-history). When a |
There was a problem hiding this comment.
I think the mention of "Workflow Function" here could be confusing, partly because of the formality implied by the uppercase F and partly because the term isn't consistent for all SDKs (such as Java, where the Workflow is defined in a method). Using "Workflow Definition" instead of "Workflow Function" would alleviate that, but that also sounds weird to me.
So, instead of this:
When a Workflow Function replays, the Commands that are emitted are compared
with the existing Event History.
Consider this:
When the Workflow's code replays, the Commands that are emitted are compared
with the existing Event History.
(Or alternatively, "When the Worker replays code in the Workflow Definition, the Commands ...")
There was a problem hiding this comment.
Yeah, I agree. I was trying to get rid of some of these old overly formal terms we insisted on having in the past but I didn't know what the feeling was on that now. Happy to do something simpler.
Sushisource
force-pushed
the
improve-determinism
branch
from
ca53a22 to
67658fd
Compare
| - However, it is not safe to change the types or IDs of Child Workflows or Activities | ||
| - The input parameters used to Signal an external Workflow | ||
| - The duration of Timers (although changing them to 0 is not safe in all SDKs) | ||
| - Add or remove calls to Workflow APIs that don't produce Commands (e.g., `workflow.GetInfo` in the Go SDK or its equivalent in other SDKs) |
There was a problem hiding this comment.
Commands is un-introduced at this point. Amounts to "know what's on this list that we haven't given you."
Not having brilliant ideas to fix, maybe say "certain read-only workflow APIs?"
If you keep the Commands concept, link out to the definition.
There was a problem hiding this comment.
I think we need to have the concept generally still. Added the link.
| - Running a SideEffect or MutableSideEffect | ||
|
|
||
| More formally, the use of certain Workflow APIs in the function is what generates | ||
| [Commands](/workflow-execution#command). Commands tell the Temporal Service which Events to create |
There was a problem hiding this comment.
As mentioned above, this is the first link but third mention.
| If a corresponding Event already exists within the Event History that maps to the generation of that Command in the same sequence, and some specific metadata of that Command matches with some specific metadata of the Event, then the Function Execution progresses. | ||
| A critical aspect of developing Workflow Definitions is ensuring they are deterministic. Generally | ||
| speaking, this means you must take care to ensure that any time your Workflow code is executed it | ||
| makes the same Workflow API calls in the same sequence, given the same input. Some changes to those |
There was a problem hiding this comment.
How about a paragraph at the top that's something like "The two most common sources of non-determinism are when workflows interact with data sources without memoizing their results, and when users deploy new code that gets executed by an existing workflow."
And then get into the details.
There was a problem hiding this comment.
I can add something about the two sources. Users don't think of Temporal as "memoizing" though, even if that is what it does.
Actually, this is addressed at the end of the section and I'm not sure how much it helps earlier before going over the concepts.
There was a problem hiding this comment.
I think it helps to have a "when/why I should care"/"give me a concrete example" section at the top to help orient folks. But up to you.
| The following Workflow API calls all can produce Commands, and thus must not be reordered, added, or | ||
| removed without proper [Versioning techniques](#workflow-versioning): | ||
|
|
||
| - Starting or cancelling a Timer |
There was a problem hiding this comment.
We have https://docs.temporal.io/references/commands so this is somewhat duplicative. (though this is more concisely readable). Wonder if we should consolidate.
There was a problem hiding this comment.
Hmm... good point. I think both formats serve a purpose. I'll link to there from here.
| and add to the Workflow Execution's [Event History](/workflow-execution/event#event-history). When | ||
| the Workflow's code [replays](/workflow-execution#replay), the Commands that are emitted are | ||
| compared with the existing Event History. If a corresponding Event already exists within the Event | ||
| History that matches that command, then the Execution progresses. |
There was a problem hiding this comment.
Not sure if you'd like to link to this for a detailed walkthrough of how this works: https://docs.temporal.io/encyclopedia/event-history/
What does this PR do?
Provide the answers people are looking for up front in simpler language before diving into details.
Notes to reviewers