Update Workflow docs editing #6571
Conversation
stephanos
force-pushed
the
update-docs-editing
branch
6 times, most recently
from
caaf58e
to
a7268d3
Compare
stephanos
force-pushed
the
update-docs-editing
branch
from
a7268d3
to
67ed1a5
Compare
docs/architecture/in-memory-queue.md
Outdated
| The in-memory queue only supports `WorkflowTaskTimeoutTask` and it only enforces | ||
| `SCHEDULED_TO_START` and `START_TO_CLOSE`. |
There was a problem hiding this comment.
There are only two timeout types for any WFT. My intent here was to emphasize that only one type of timer tasks (WorkflowTaskTimeoutTask) is supported which enforce both timeouts. In your version, it sounds like normal WFT has other timeouts but for speculative we have only these 2.
There was a problem hiding this comment.
what's the other timeout type?
There was a problem hiding this comment.
For activity tasks there is also SCHEDULE_TO_CLOSE which is for all attempts. WFT doesn't have it, it is getting retried forever.
There was a problem hiding this comment.
Gotcha! Changed it to this now:
The in-memory queue only supports
WorkflowTaskTimeoutTask, and enforces the
SCHEDULE_TO_STARTandSTART_TO_CLOSEtimeouts.
|
|
||
| > #### TODO | ||
| > In fact, this is not used. Server always set `event_id` equal to event Id before `WorkflowTaskStartedEvent`, |
There was a problem hiding this comment.
I think it is important to indicate that this field is not currently used at all. Yes, from your version it kinda implies that, but I would like to see clear sentence saying "it doesn't work this way".
There was a problem hiding this comment.
I think I found the "not used part" confusing. It sounds like the worker isn't using the field? I've updated the text now.
There was a problem hiding this comment.
I see. Yes, event_id is set by server and respected by all SDK. It just set not to intended value. command_index though is indeed not used at all: it is not set by SDKs (at least go) and not checked by server.
There was a problem hiding this comment.
That makes sense 👍
| When Workflow Task finally completes, `WorkflowTaskScheduled` and `WorkflowTaskStarted` events | ||
| are getting written to history followed by `WorklfowTaskCompleted` event. | ||
| Every Workflow Task ships history events to the worker. It must always contain the two events | ||
| `WorkflowTaskScheduled` and `WorkflowTaskStarted`. There might be some events in-between them if |
There was a problem hiding this comment.
I believe it is important to say that WFTStarted must be the last (not just contains).
There was a problem hiding this comment.
I removed that part because the very next sentence contradicts the original statement that the last two events are WFTScheduled and WFTStarted. I'll change it to emphasize that WFTStart must be last.
| the server writes a corresponding Workflow Task failed event to the history and increases the | ||
| attempt count in the mutable state. | ||
|
|
||
| For the next attempt, a **transient Workflow Task** is used: the Workflow Task events |
There was a problem hiding this comment.
So... the is idea here was that there is no such thing as transient WFT. All WFT are the same. There are transient events. And we call WFT with such events "transient WFT" just for convenience instead of saying "WFT with transient events". I think this idea is lost in your version, although it still describes it pretty well.
Co-authored-by: Alex Shtin <[email protected]>
Co-authored-by: Alex Shtin <[email protected]>
Co-authored-by: Alex Shtin <[email protected]>
Co-authored-by: Dan Davison <[email protected]>
| the response from the `RecordWorkflowTaskStarted` API. The worker does not know they are transient, | ||
| though. If the Workflow Task keeps failing, the attempt counter is increased in the mutable state, | ||
| and transient Workflow Task events are created again - but no new failure event is written into the | ||
| history again. When the Workflow Task finally completes, the `WorkflowTaskScheduled` and |
There was a problem hiding this comment.
Now you removed transient WFT completely :-). But we do use this name in code and comments. I would add an original sentence here:
Workflow Task, which has transient Workflow Task events, is called ***transient Workflow Task***.
|
Thanks for clearing and clarifying everything in the docs. |
Co-authored-by: Alex Shtin <[email protected]>
What changed?
Edited remaining Update Workflow docs not already covered by #6486.