Skip to content

time skipping supports max skip, polling, describe - #835

Open
feiyang3cat wants to merge 20 commits into
mainfrom
vts-maxskip-poll
Open

time skipping supports max skip, polling, describe#835
feiyang3cat wants to merge 20 commits into
mainfrom
vts-maxskip-poll

Conversation

@feiyang3cat

@feiyang3cat feiyang3cat commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

What changed?

  1. add a max skip field to TimeSkippingConfig
  2. add TimeSkippingInfo to DescribeWorkflowExecution (contains virtual current time and time-skipping runtime status)
  3. add PollWorkflowExecutionTimeSkipping for fast-forward completion
  4. comment refinements: remove the workflow keyword from time-skipping message comments under the common path
  5. change fast_forward from a duration field to a message with both duration and id

Breaking changes
Change #5

Server PR
temporalio/temporal#11220

google.protobuf.Timestamp fast_forward_target_time = 2;

// The initial skip count. It only propagates across a chain of runs within the same execution.
int32 initial_skip_count = 3;

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.

for reviewers: this shall be propagated so that we can stop workflow retry as workflow retry generates a new run instead retrying within the same run

@feiyang3cat feiyang3cat changed the title add max_skip_count and change comments of TimeSkippingConfig time skipping supports max skip, polling, describe Jul 20, 2026
Comment thread temporal/api/common/v1/message.proto Outdated
//
// If this field is not set, the server applies a large default value (e.g. 100). The default can
// be changed through dynamic config, and is overridden by this field when set.
int32 max_skip_per_session = 4;

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.

we use a per_session max_skip instead of a total/delta max_skip to make this feature easy

the unneeded complexity introduced by other options:

  1. total: when the value is updated, both the client and server need to read current skip number to decide if the new value is valid and we may need to expose the current skip number
  2. delta: as we set a default value when the field is not set, if the user set TSC multiple times, delta may silently accumulate the total number to a large number

// The fast-forward that was active when the poll started reached its target time and completed.
RESULT_FAST_FORWARD_COMPLETED = 2;
// No fast-forward was in progress when the poll started, so there was nothing to wait for.
RESULT_NO_PENDING_FAST_FORWARD = 3;

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.

also need to add an result of workflow end (status = completed/failed/canceled/TERMINATED/TIMED_OUT)
but we need the whole execution (a chain of runs) to end instead of a single run's status


message PollWorkflowExecutionTimeSkippingResponse {
enum Result {
// The poll timed out server-side before any time-skipping state change occurred to fast forward.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sound like this should be different than RESULT_UNSPECIFIED, maybe RESULT_TIMED_OUT?

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.

udpated

Comment thread temporal/api/common/v1/message.proto Outdated
// In-flight work includes activities, child workflows, Nexus operations, signal/cancel external workflow operations, etc.
// User timers are not classified as in-flight work and will be skipped over; the virtual clock may also skip to the
// time point of the registered fast-forward when there is no in-flight work.
// Every time time is skipped, the skip count is incremented by one; max_skip_per_session bounds the number of skips allowed within a single time-skipping session.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

time time :)

@feiyang3cat
feiyang3cat force-pushed the vts-maxskip-poll branch 5 times, most recently from 2a371d8 to dd493a2 Compare July 21, 2026 00:41
Comment thread temporal/api/common/v1/message.proto Outdated
google.protobuf.Duration fast_forward = 2;

// A client-supplied ID that must be set together with `fast_forward`. It is used to poll for
// fast-forward completion via PollWorkflowExecutionTimeSkipping.

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.

for reviewers: explicitly making this id required if fast_forward is used for simplicity

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a reason not to make it optional? The SDK only needs this to wait on the result of a FF, and it doesn't always need to do that.

@feiyang3cat feiyang3cat Jul 29, 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.

I think both ways work:

  1. server doesn't allow empty ff-id, and if users don't want to wait on it they can set a default id or random id

  2. server allow setting empty ff-id, and we need to define a sound behavior for polling with ff-id empty. I think the simplest way is we don't allow polling an empty ff-id

    • 2.1. so polling with ff-id in the request will have an argument error
    • 2.2. polling with a valid ff-id but the current ff-id is empty will return ff-id not matching
    • 2.3. in comments we indicate if users want to poll for the ff they need to set a ff-id for it

Comment thread temporal/api/common/v1/message.proto Outdated
// If the execution is actively trying to skip time automatically when there is a chance,
// this field will be set to true. If time has stopped skipping either by fast-forward completion,
// max skip allowed checking, or user configuration, it will be false.
bool is_running = 2;

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.

@feiyang3cat
feiyang3cat force-pushed the vts-maxskip-poll branch 3 times, most recently from d9062aa to 226b1a5 Compare July 23, 2026 01:39
FastForwardConfig fast_forward_config = 2;

// By default, executions started by another execution (e.g. a child workflow of a parent workflow or
// a schedule with the timeskipping policy enabled), inherit the "enabled" flag and skip time when possible.

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.

refine the comment: this is about config propagation for different executions (not different runs within the same execution

// This flag disables that inheritance.
bool disable_propagation = 3;

// The maximum number of skips allowed every time this field is updated. It protects the execution from

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.

Comment thread temporal/api/common/v1/message.proto Outdated
google.protobuf.Duration fast_forward = 2;

// A client-supplied ID that must be set together with `fast_forward`. It is used to poll for
// fast-forward completion via PollWorkflowExecutionTimeSkipping.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a reason not to make it optional? The SDK only needs this to wait on the result of a FF, and it doesn't always need to do that.

// The fast-forward that was active when the poll started reached its target time and completed.
// The poll timed out server-side before the fast-forward completed. The caller may poll again.
RESULT_POLL_TIMEOUT = 1;
// The fast-forward identified by the request's `fast_forward_id` reached its target time and completed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe repeat here that the server only keeps the most recently started FF ID.

// max_session_skip_count, time skipping stops. Whenever this config field is updated, the accumulated
// skip count is cleared, marking the start of a new session.
// For an execution with a chain of runs (retry, cron, continue-as-new), the count is accumulated
// across all runs within the same session.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I wondering about an edge case where you execute a single fast forward, but across e.g. 3 retries: does this count as 3 skips against the max, or just one?

If it's just one, perhaps add something like "multiple retries during a single fast-forward only count as one skip".

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.

it will count as 3, and the default value right now is 250. I think it will be fine users can change the default value or override the value using api. But indeed it is hard for user to estimate the number accurately if they don't want to study the internal server timer mechanism so it will be rough number.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How about a workflow that does 3 multiple sequential sleeps? Does each one count against the max?

I realize that this field is probably not one that users will set very often, but I want to figure out a way to write a test for it, if the SDK is going to expose it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Or perhaps it is enough to test this by setting the field, and reading the configuration back and confirming it? Rather than verifying server behavior?

@feiyang3cat feiyang3cat Jul 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. skipping 3 sleeps will count as 3. server won't skip multiple timers at each skip.

  2. I agree that users most of the time don't need to feel the existence of this option. And even if this max skip takes effect, they may not want to set this field but to figure out if their tests have unlimited retires.

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.

3 participants