Skip to content

automatic time-skipping (don't merge)#740

Open
feiyang3cat wants to merge 2 commits intomasterfrom
api-time-skipping-v2
Open

automatic time-skipping (don't merge)#740
feiyang3cat wants to merge 2 commits intomasterfrom
api-time-skipping-v2

Conversation

@feiyang3cat
Copy link

What changed?
new feature: automatic time-skipping configuration and new event type added

Breaking changes
new event type added but right now no workflow history will have this new event type

Server PR
not expect to break server, no new rpc-handler and new fields are all optional

@feiyang3cat feiyang3cat requested review from a team as code owners March 23, 2026 22:25
@feiyang3cat feiyang3cat changed the title POC automatic timeskipping automatic time-skipping Mar 23, 2026
message TimeSkippingConfig {

// If set, enables automatic time-skipping for this workflow execution.
// It can also be disabled by setting this field to false.
Copy link
Author

@feiyang3cat feiyang3cat Mar 23, 2026

Choose a reason for hiding this comment

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

@Sushisource Hey! I've cleaned this pr and ready for review.

And this config may need special attention since it is changed significantly from previous version Chad proposed.
(1)deleted most of the fine-grained control (like max timer-firing count, max duration to skip) from the previous version. The reason is it is ambiguous when complicated configuration is propagated to child-workflows.
(2) I have also changed the nested config to one single layer because this configuration is only used for automatic time-skipping, manual time-skipping will have a separate request and response payload
(3) we allow disabling time-skipping for in-flight workflows since it is not hard for the server to have this feature, and it will be a nice-to-have feature same as the SDK TestServer.

I think we can consider wrapping up this pr early, and put manual time-skipping in a separate pr maybe.
Pls feel free to disagree, happy to refine.

Copy link
Member

Choose a reason for hiding this comment

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

I think this looks totally reasonable - but I also don't see any reason to rush the PR while work is in progress unless we intend to ship a version without manual skipping first. If we don't, let's use the opportunity to develop it and decide if we want to make any changes to this config that relate to manual timeskipping.

@feiyang3cat feiyang3cat force-pushed the api-time-skipping-v2 branch 2 times, most recently from 500a020 to 83f6639 Compare March 23, 2026 23:16
message TimeSkippingConfig {

// If set, enables automatic time-skipping for this workflow execution.
// It can also be disabled by setting this field to false.
Copy link
Member

Choose a reason for hiding this comment

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

I think this looks totally reasonable - but I also don't see any reason to rush the PR while work is in progress unless we intend to ship a version without manual skipping first. If we don't, let's use the opportunity to develop it and decide if we want to make any changes to this config that relate to manual timeskipping.

@feiyang3cat feiyang3cat changed the title automatic time-skipping automatic time-skipping (don't merge) Mar 24, 2026
@feiyang3cat feiyang3cat force-pushed the api-time-skipping-v2 branch from 862f8a0 to 4aeceb6 Compare March 25, 2026 06:37
@feiyang3cat feiyang3cat force-pushed the api-time-skipping-v2 branch from 4aeceb6 to 094e2c5 Compare March 25, 2026 06:40
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.

2 participants