Skip to content

clean up todo comments of time skipping - #12046

Merged
feiyang3cat merged 2 commits into
temporalio:mainfrom
feiyang3cat:vts-clean-todo
Sep 15, 2026
Merged

feiyang3cat merged 2 commits into
temporalio:mainfrom
feiyang3cat:vts-clean-todo

Conversation

@feiyang3cat

@feiyang3cat feiyang3cat commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

What changed?

  1. check and clean all todos of time skipping for workflows
  2. fix bugs of wrapTimeSourceWithTimeSkipping in passive cluster

Why?

when those features were added, todo comments are not delete together

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

`

)
}

if s.executionInfo.GetTimeSkippingInfo() != nil {

@feiyang3cat feiyang3cat Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this executionInfo is a fresh one and always have nil timeSkippingInfo

the scenarios we need to wrap time source

  1. when an execution is loaded from DB -> NewMutableStateFromDB
  2. when a new child/next run has initial skipped duration propagated ->
  • func (t *transferQueueActiveTaskExecutor) startChildWorkflow
  • func NewMutableStateInChain(
  1. when an execution has time skipping enabled at start time
  2. when an execution has time skipping enabled in the middle of its execution

-> both 3&4 ApplyWorkflowExecutionOptionsUpdatedEvent → TimeSkippingInfo == nil → initTimeSkippingInfo → wrapTimeSourceWithTimeSkipping

@feiyang3cat
feiyang3cat marked this pull request as ready for review September 13, 2026 19:17
@feiyang3cat
feiyang3cat requested review from a team as code owners September 13, 2026 19:17
@feiyang3cat
feiyang3cat requested a review from simvlad September 13, 2026 19:17
@feiyang3cat
feiyang3cat merged commit 79be1c4 into temporalio:main Sep 15, 2026
90 of 92 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants