api changes for time-skipping propagation#770
Conversation
feiyang3cat
force-pushed
the
api-ts-patch-2
branch
from
bcf6ece to
be3ff37
Compare
| @@ -590,28 +590,28 @@ message WorkflowExecutionOptions { | |||
| // User timers are not classified as in-flight work and will be skipped over. | |||
| // When time advances, it skips to the earlier of the next user timer or the configured bound, if either exists. | |||
| message TimeSkippingConfig { | |||
There was a problem hiding this comment.
these reserved ones can be removed
but buf-breaking step in Makefile:115-117 stops this kind of changes
feiyang3cat
force-pushed
the
api-ts-patch-2
branch
2 times, most recently
from
d5b3984 to
b9d4b1b
Compare
and add propagated_skipped_duration to propagate virtual time
feiyang3cat
force-pushed
the
api-ts-patch-2
branch
from
b9d4b1b to
bc98d6b
Compare
feiyang3cat
force-pushed
the
api-ts-patch-2
branch
from
bc98d6b to
332d275
Compare
feiyang3cat
changed the title
| // and possibly other features added in the future. | ||
| // User timers are not classified as in-flight work and will be skipped over. | ||
| // When time advances, it skips to the earlier of the next user timer or the configured bound, if either exists. | ||
| // |
There was a problem hiding this comment.
cc @yycptt added here but yet still this cannot replace the power of an educational document with examples in our web
feiyang3cat
force-pushed
the
api-ts-patch-2
branch
from
5748637 to
8c1f458
Compare
|
|
||
| // If this execution was started by a previous execution that had already skipped some time, | ||
| // it inherits the accumulated skipped duration from that execution through this field. | ||
| // This field is set internally by the server and cannot be configured by the user. |
There was a problem hiding this comment.
Should this even be here then? Maybe it should be a separate field on StartWorkflowExecutionRequest and WorkflowExecutionStartedEventAttributes. Then you can also avoid all those packing/unpacking/cloning changes.
There was a problem hiding this comment.
oh, yes. I think this makes sense. Let me change it.
feiyang3cat
force-pushed
the
api-ts-patch-2
branch
2 times, most recently
from
1b2fb47 to
8866718
Compare
… start child attr
feiyang3cat
force-pushed
the
api-ts-patch-2
branch
from
8866718 to
dd9b254
Compare
| // The propagated time-skipping configuration for the child workflow. | ||
| temporal.api.workflow.v1.TimeSkippingConfig time_skipping_config = 21; | ||
|
|
||
| // Propagate the duration skipped to the child workflow. |
There was a problem hiding this comment.
need snapshot for child workflow as the parent may still be running
but not needed for CaN as the CaN Event is the closing event of the previous wf
| // If a workflow execution is started by a previous execution (parent-child workflow or continue-as-new) | ||
| // that has already skipped some time, the accumulated skipped duration from that execution | ||
| // can be passed to the new workflow execution through this field. | ||
| google.protobuf.Duration initial_skipped_duration = 30; |
There was a problem hiding this comment.
Do we need this in this request proto? We have internal proto definition for the history service's StartWorkflowExecutionRequest. If SDK's doesn't have to provide this value, we can move this to that proto. For the use cases of StartChildWorkflow and Continue-As-New, we only call this API on history service without going through frontend.
There was a problem hiding this comment.
I see. this makes sense
feiyang3cat
force-pushed
the
api-ts-patch-2
branch
from
14b8f1c to
a6122c2
Compare
3 participants
What changed and why?
Propagation refers to how time-skipping config is carried over to all workflows started by the current workflow: retry, continue-as-new, child workflows, reset, etc.