temporal icon indicating copy to clipboard operation
temporal copied to clipboard

Expose history size to workflows

Open dnr opened this issue 3 years ago • 1 comments

What changed? This fills in HistorySizeBytes and SuggestContinueAsNew on WorkflowTaskStartedEventAttributes, added in https://github.com/temporalio/api/pull/178.

Setting as draft until I remove the // QUESTION comments.

Why? So workflows can decide whether to continue-as-new with less guessing. Fixes #2726 and #1114

How did you test it? New integration test

Potential risks Bugs in this logic could lead to inconsistency between the values sent in transient workflow tasks and values actually recorded in history, which could lead to determinism errors on replay.

Is hotfix candidate? no

dnr avatar Jul 05 '22 16:07 dnr

I think we only need the values in WorkflowTaskStartedEventAttributes. Could you explain what is purpose of the values used in ExecutionInfo, and what the purpose of it in WorkflowTaskInfo.

It's an obscure corner case, but I thought in the discussion we decided we needed to handle it:

What if you send the worker a wftstartedevent with suggestcontinueasnew == false, and it fails/times out. Then you send a second attempt, which is now a transient wft, with suggestcontinueasnew == false. Then you change dynamic config so that the same history size now makes suggestcontinueasnew == true. Now the worker responds to the wft successfully, and you have to write out the transient events to history. If you re-evaluate suggestcontinueasnew at that point and write a wftstartedevent with it as true, you'll get a determinism error on replay. (Assuming the workflow follows the suggestion.)

If we didn't use dynamic config I agree we wouldn't have to keep it in mutable state.

dnr avatar Jul 18 '22 18:07 dnr

Feb,9-Jul,5 = 219

alexshtin avatar Feb 09 '23 20:02 alexshtin