Skip to content

move workflow time skipping code into a different file#10867

Open
feiyang3cat wants to merge 8 commits into
temporalio:mainfrom
feiyang3cat:ts-rf-workflow
Open

move workflow time skipping code into a different file#10867
feiyang3cat wants to merge 8 commits into
temporalio:mainfrom
feiyang3cat:ts-rf-workflow

Conversation

@feiyang3cat

@feiyang3cat feiyang3cat commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

What changed?

  1. move time skipping functions from mutable_state_impl.go to time_skipping.go and add sectional comments
  2. refinements
    • simplify workflow time skipping check to one function isWorkflowSkippable
    • renaming methods: add workflow to relevant methods to separate them from chasm methods
    • make TimeSkippingTransition as a standard data structure and will be used by chasm as well
    • add nil check to avoid panics in propagation methods

Why?

  1. mutable_state_impl.go is over 10k lines and hard to navigate
  2. preparation for chasm executions

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)

@feiyang3cat feiyang3cat requested review from a team as code owners June 28, 2026 04:39
"google.golang.org/protobuf/types/known/timestamppb"
)

// =============================================================================

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.

added sectional comments

// =============================================================================
// Time Skipping Runtime Data Structure
// =============================================================================
type timeSkippingTransition struct {

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.

expose the new and methods of this struct so that chasm framework can use this data structure directly

// if checks if time skipping is enabled, if the workflow has in-flight work,
// and if the workflow is at the correct state and status to skip time.
// And if there is a time point to skip to is not the scope of this method.
func (ms *MutableStateImpl) isWorkflowSkippable() bool {

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.

merged hasInflightWorkToPreventTimeSkipping and shouldExecuteTimeSkipping into this unified method

transition.TrackEarliestFutureTime(executionTime)
}
}
if ms.HadOrHasWorkflowTask() {

@feiyang3cat feiyang3cat Jun 28, 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.

  1. this is the key logic change to allow execution/run timeout

  2. but the if condition for status check is added because child workflows

Why root workflows don't have this problem:
the create transaction both applies the start event and calls AddFirstWorkflowTaskScheduled. So when that transaction's close-tx runs closeTransactionHandleWorkflowTimeSkipping, the workflow already has a pending WFT → HasPendingWorkflowTask() == true → isWorkflowSkippable() returns false ("has pending workflow task"). There is no window where a root workflow is running, time-skipping-enabled, and idle with no WFT. So the timeout target is never reached on a not-yet-started root workflow — the HadOrHasWorkflowTask guard changes nothing for it.

Why child workflows have the problem:
child workflow GenerateFirstWorkflowTask returns 0, nil — it does not schedule the first WFT. The child MS is created and persisted with only the start event; the first WFT is scheduled later in a separate step (the scheduleworkflowtask API path, driven after the parent's transfer task records the child as started). In between:

  • running ✓, time-skipping enabled ✓ (config propagated from parent),
  • no pending WFT (not scheduled yet), no completed WFT → HadOrHasWorkflowTask() == false,
  • no timers / activities / children → isWorkflowSkippable() returns true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant