Proto changes to support activity pause by type options #613

gow · 2025-07-11T00:15:49Z

We decided to support pausing of activities by type with the following options:

All: Pause all running and future activities of the given type.
Running: Pause just the currently running activities.
Future: Pause just the future activities if the given type.

This is to support different options while pausing by activity type.

cretz · 2025-07-11T17:52:55Z

temporal/api/workflowservice/v1/request_response.proto

    string reason = 6;
+
+    // (-- api-linter: core::0123::resource-annotation=disabled --)
+    message PauseByType{


Pedantic, but usually there is a space between identifier and open brace

cretz · 2025-07-11T17:54:03Z

temporal/api/workflowservice/v1/request_response.proto

+        PauseByTypeOption option = 2;
+    }
+
+    enum PauseByTypeOption{


I was under the impression we were considering visibility-like queries for deciding what activities certain actions would affect, is that the case?

This is for activity pause/unpause APIs. The primary purpose is to allow someone to pause all activities of a certain type. The current implementation explicitly pauses just the running instances of an activity and doesn't pause any future instances.
Regarding rules & actions - nothing changes. Rules will still use queries and identify the activities (by id) to be paused.

This is for activity pause/unpause APIs

Mentioned in other comment, but this is not applying to unpause that I can see

The primary purpose is to allow someone to pause all activities of a certain type

What if I want to pause all activities regardless of type? In unpause that is a boolean, not here?

The current implementation explicitly pauses just the running instances of an activity and doesn't pause any future instances.

So how can I pause future instances without being forced to provide a type? Or pause all?

We should be consistent here. Why does whether I want my predicate to apply to all or running or future have to do with what that predicate is?

Are we conflating rules and individual activity operations here? My understanding of these individual activity operations is that they operate on specified activity/s. Activities are identified by id or by type in the API input. When identified by id it's implicit that they are currently running activities (so no additional options apply here). When identified by type they could be currently running or the ones started in future. So we need additional options (running, future, all). So actions differ based on how you identify the activity.

So how can I pause future instances without being forced to provide a type?

If we want to do this via activity pause/unpause APIs we can support wildcard * to identify all activities? The question is how do we identify/represent "all activities" in these APIs.

Workflow rules have predicates that are evaluated and automated action is taken. This is were typically actions are independent of predicates.
Let me know if my understanding of how these APIs and rules would be employed is incorrect.

My understanding of these individual activity operations is that they operate on specified activity/s.

Why can't I apply activity operations on all activities for a workflow? Why are we limiting to only certain IDs or types?

When identified by id it's implicit that they are currently running activities (so no additional options apply here). When identified by type they could be currently running or the ones started in future.

Regardless of how identified (id, type, all, selector in the future, etc), I should be able to specify an activity state they should apply to.

Can I pause all running activities? Can I pause all future activities? Can I pause all activities regardless of whether future or running?

If we want to do this via activity pause/unpause APIs we can support wildcard * to identify all activities?

No, unpause already has this, a boolean to apply to all. We're strangely inconsistent with ourselves on how to apply something to all activities (or all running or all future). We can become consistent by adding a boolean here if we want for "all" or we can change that one.

cretz · 2025-07-11T17:54:37Z

temporal/api/workflowservice/v1/request_response.proto

+        PauseByTypeOption option = 2;
+    }
+
+    enum PauseByTypeOption{


Might I want to choose all, running, or future for more than just activity type? For instance, how do I pause all future activities regardless of type? Whether the predicate applies to all, running, or future doesn't seem to relate to the predicate itself.

These are part of single activity operation APIs (pause/unpause/reset). Pausing all activities within a workflow is under the scope of workflow pause (which is being speced currently).

What about only pausing future activities regardless of type? It is confusing to only be able to do some activity pausing via workflow pause. I don't want to pause my workflow, I want to pause activities. I think we need to think about this from a devx/user POV. I want to pause all activities, I want to pause running activities, I want to pause future activities, I want to pause running activities of a certain type, etc etc need to be a cohesive, easy to understand interface. To tell someone half of that is done elsewhere is not good devx.

Agree. All cases you mentioned are covered except being able to specify "all" activity types. Like mentioned above, we can add a wildcard * to allow users to indicate that they want to pause all activities. Thoughts?

Mentioned in other reply, we should be consistent with unpause. Why have a boolean for all there and a special string for all here? And why should I even have to set a type at all if I don't care about type?

cretz · 2025-07-11T17:55:51Z

temporal/api/workflowservice/v1/request_response.proto

+        PauseByTypeOption option = 2;
+    }
+
+    enum PauseByTypeOption{


Unpause has a concept of type also, does this same concern apply there? Also, unpause has a concept of "all" there too, does this same feature need to apply here?

No. I'm not planning to support selectively unpausing activity by type. We can add it in the future if needed?

We already support unpausing activity by type today, see the API. So today we support pausing and unpausing activities with the same predicate capabilities. This now makes pause selection different than unpause selection and therefore confusing from a user understanding POV.

I agree about the inconsistency between pause and unpause API. My initial reasoning was that there is no use case to selectively unpause just running, future, all (when unpausing by type). So I left it out. I'll add similar unpause options to unpause API as well.

I'll add similar unpause options to unpause API as well.

👍 Will be interested in what this looks like when done. I think you'll find that the enumerate state of the activity is wholly unrelated to its type and therefore PauseByType doesn't make sense because it doesn't sense to combine the two since they are distinct filters.

I see your point. Added PauseOptionActivityState (open to other name suggestions). Now one can pause/unpause any combination of

activity: {id, type, all}

activity_state: {running, future, both}

There are some invalid combinations like {activity: id, activity_state: future}. But these will be handled in the API implementation.

This looks great to me!

This reverts commit 441dcd3.

cretz

LGTM, but can we make sure to at least get most of the impl done server side before we merge this? I want to confirm the server can work with these changes and we won't learn something during implementation.

ychebotarev · 2025-07-15T15:59:28Z

temporal/api/workflowservice/v1/request_response.proto

    // Reason to pause the activity.
    string reason = 6;
+
+    // which activities to pause - current, future or both.


comment says "which activities to pause" - but previous oneof selector selects which activities to pause.
also what is the interaction between id and activity_state? What does it mean 'future activities by id'?

same for "match all". I don't think "match all" and "for all, now and in the future" should coexist...
hm. I think you separated those qualifiers into activity_state after discussion with Chad, but I don't think this is the right idea.

it should be part of "type".
(not going to block on this)

IMO they are independent filters and if both are present, both must be satisfied. The other is "which activities", this is "also limit by current state" IMO.

bergundy

I thought we said that we want pause for future activities to be done globally in namespace rules. Surprised to see us adding per-workflow future activity pause.

bergundy · 2025-07-15T16:29:08Z

temporal/api/enums/v1/workflow.proto

 }
+
+// Pause option activity state is used to specify if we want to pause/unpause just the current activity, or just the future activities or both.
+enum PauseOptionActivityState {


Why is this in workflow.proto and not activity.proto?

Or maybe this could be generalized for workflows too? Would we want this for workflow pause?

bergundy · 2025-07-15T16:54:41Z

temporal/api/enums/v1/workflow.proto

+    PAUSE_OPTION_ACTIVITY_STATE_UNSPECIFIED = 0;
+    PAUSE_OPTION_ACTIVITY_STATE_RUNNING = 1;
+    PAUSE_OPTION_ACTIVITY_STATE_FUTURE = 2;
+    PAUSE_OPTION_ACTIVITY_STATE_BOTH = 3;


Suggested change

PAUSE_OPTION_ACTIVITY_STATE_BOTH = 3;

PAUSE_OPTION_ACTIVITY_STATE_ANY = 3;

ychebotarev · 2025-07-15T17:00:24Z

temporal/api/enums/v1/workflow.proto

 }
+
+// Pause option activity state is used to specify if we want to pause/unpause just the current activity, or just the future activities or both.
+enum PauseOptionActivityState {


either create acitvity.proto, or add to "common".
Or remove "activity" and change it to "ENTITY" to generalize?

why this comment is here...

cretz · 2025-07-15T17:45:07Z

temporal/api/workflowservice/v1/request_response.proto

    // If set, the activity will start at a random time within the specified jitter duration.
    google.protobuf.Duration jitter = 9;
+
+    // which activities to unpause - current, future or both.


Inconsistent with capitalization of comments

cretz · 2025-07-15T17:45:32Z

temporal/api/workflowservice/v1/request_response.proto

    // If set, the activity will start at a random time within the specified jitter duration.
    google.protobuf.Duration jitter = 9;
+
+    // which activities to unpause - current, future or both.


Safe to assume "any" is the default when unspecified? Can you clarify that in comment?

Proto changes to support activity pause by type options

441dcd3

gow requested review from a team as code owners July 11, 2025 00:15

gow requested review from drewhoskins-temporal, spkane31 and ychebotarev July 11, 2025 00:16

cretz reviewed Jul 11, 2025

View reviewed changes

gow added 3 commits July 14, 2025 14:41

Revert "Proto changes to support activity pause by type options"

d7a5344

This reverts commit 441dcd3.

Use PauseOptionActivityStatus

7b363d9

PauseOptionActivityStatus -> PauseOptionActivityState

53a2159

cretz approved these changes Jul 15, 2025

View reviewed changes

ychebotarev reviewed Jul 15, 2025

View reviewed changes

bergundy reviewed Jul 15, 2025

View reviewed changes

ychebotarev reviewed Jul 15, 2025

View reviewed changes

cretz reviewed Jul 15, 2025

View reviewed changes

spkane31 mentioned this pull request Jul 30, 2025

pause: add pause activity information to workflow describe cmd temporalio/cli#837

Open

2 tasks

spkane31 removed their request for review October 8, 2025 15:32

	PAUSE_OPTION_ACTIVITY_STATE_BOTH = 3;
	PAUSE_OPTION_ACTIVITY_STATE_ANY = 3;

Proto changes to support activity pause by type options #613

Are you sure you want to change the base?

Proto changes to support activity pause by type options #613

Uh oh!

Conversation

gow commented Jul 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bergundy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

cretz Jul 14, 2025 •

edited

Loading

cretz Jul 11, 2025 •

edited

Loading

cretz Jul 15, 2025 •

edited

Loading