Update Workflow and Activity Priority #610

stephanos · 2025-07-08T02:33:45Z

READ BEFORE MERGING: All PRs require approval by both Server AND SDK teams before merging! This is why the number of required approvals is "2" and not "1"--two reviewers from the same team is NOT sufficient. If your PR is not approved by someone in BOTH teams, it may be summarily reverted.

What changed?

Added Priority field to the workflow and activity update API options.

Batch API support is going to be added in a follow-up PR.

Why?

Allow users to update priority after the workflow/activity was submitted.

Breaking changes

See field deprecation.

Server PR

temporalio/temporal#8396

stephanos · 2025-07-30T20:07:04Z

temporal/api/workflow/v1/message.proto


-
    // Priority metadata
+    // Deprecated. Use `activity_options.priority`.


This is a unfortunate since I only now realized that PendingActivityInfo contains ActivityOptions but we've already added temporal.api.common.v1.Priority priority = 22; there ... 😞

We need to make sure this value keeps getting written by server for a while.

Should also use [deprecated = true].

Alternatively, related to my other comment, maybe we clarify that this field is always the original priority, and activity_options has the override, if any?

Sushisource · 2025-08-01T17:32:45Z

temporal/api/activity/v1/message.proto

+    // Overrides the activity's priority sent by the SDK.
+    temporal.api.common.v1.Priority priority = 7;


This field has two meanings, one when writing, one when reading. This comment only describes the write behavior.

The read behavior might be an override or the original.

I like that! I'll change it to that.

@dnr we could lean even more into this and make the field in PendingActivityInfo always represent the user set value and ActivityOptions to contain the latest value. Meaning, when it inherits the workflow's priority, the data would show that the user set no priority but the actual priority was the one from the workflow (or a manual override).

Semantically, this would be a breaking change. Maybe fine in pre-release, though.

I sorta like the idea of differentiating what was explicitly set in on the activity from what it inherited from the workflow... and from what it was later overridden to? There are really four values: 1. inherited from workflow, 2. explicitly set on activity by workflow, 3. overridden by update call, and 4. "effective" priority which is the merged value of the first three in that order.

But it's not obvious to me why PendingActivityInfo would have one of those and ActivityOptions would have another. I mean, if you asked me I probably would have said ActivityOptions should have the explicit set value and PendingActivityInfo would have the "effective" one (backwards from what you said I think).

So if we really want to split them out, maybe we should include multiple Priorities in each of those locations? But before we get into that complexity, let's consider if it's really worth it. Why do users want to read this value and which one are they more interested in?

The most intuitive to me is: this field is 2+3 in my initial list: the explicitly-set Priority, which can be changed by update call. The "effective" value is not reported explicitly anywhere. But I could easily be convinced otherwise.

Coming back here, I think we have two options:

(a) Deprecate the field in PendingActivityInfo; keep the field in ActivityOptions. Until we can remove it, hydrate both from the server with the explicitly set data (ie 2+3); ie status quo.

(b) As proposed above, make the ActivityOptions reflect the explicitly set data (ie 2+3); and then I'd argue that realistically leaves only (4) for PendingActivityInfo since (1) seems not helpful.

I like (b) only because it would allow us to easily show what the effective priority options are in the CLI/UI. That in itself seems useful to communicate. But that would technically be a breaking behavior change; although, I don't think anyone relies on this (not sure it's exposed?). And it's pre-release.

If we don't find that valuable (or possibly confusing; the biggest risk here AFAICT), we still have option (a) since the feature is in pre-release.

Looking again with a fresh perspective: it seems like anything other than (a) (duplicate in both places, maybe deprecate the old one) is inevitably going to confuse someone badly, and I would just go with that. PendingActivityInfo is always understood in the context of a workflow, right? So anyone who sees a PendingActivityInfo can compute the effective Priority. (Standalone activities have no workflow to inherit from so that's fine too.) If we want to expose effective priority to avoid duplicating that logic, let's do it in a new field.

(Re-reading comments, that basically matches what I said on Aug 4, which is good)

Sushisource · 2025-08-01T17:33:22Z

temporal/api/workflow/v1/message.proto


-
    // Priority metadata
+    // Deprecated. Use `activity_options.priority`.


We need to make sure this value keeps getting written by server for a while.

Should also use [deprecated = true].

Alternatively, related to my other comment, maybe we clarify that this field is always the original priority, and activity_options has the override, if any?

dnr · 2025-08-04T22:59:10Z

temporal/api/activity/v1/message.proto

+    // Overrides the activity's priority sent by the SDK.
+    temporal.api.common.v1.Priority priority = 7;


I sorta like the idea of differentiating what was explicitly set in on the activity from what it inherited from the workflow... and from what it was later overridden to? There are really four values: 1. inherited from workflow, 2. explicitly set on activity by workflow, 3. overridden by update call, and 4. "effective" priority which is the merged value of the first three in that order.

But it's not obvious to me why PendingActivityInfo would have one of those and ActivityOptions would have another. I mean, if you asked me I probably would have said ActivityOptions should have the explicit set value and PendingActivityInfo would have the "effective" one (backwards from what you said I think).

So if we really want to split them out, maybe we should include multiple Priorities in each of those locations? But before we get into that complexity, let's consider if it's really worth it. Why do users want to read this value and which one are they more interested in?

The most intuitive to me is: this field is 2+3 in my initial list: the explicitly-set Priority, which can be changed by update call. The "effective" value is not reported explicitly anywhere. But I could easily be convinced otherwise.

dnr · 2025-08-04T22:59:47Z

temporal/api/history/v1/message.proto

    string attached_request_id = 3;
    // Completion callbacks attached to the running workflow execution.
    repeated temporal.api.common.v1.Callback attached_completion_callbacks = 4;
+    temporal.api.common.v1.Priority priority = 5;


Maybe a comment here? Does this contain the full new Priority or only the changed fields?

👍 I've added a comment now

Since this is for the workflow options, it's valid to allow zero values in the update (e.g. for fairness key). Therefore we must store the full priority message even if the user only updated one field to disambiguate between default/not changed and a change.

Do we have to change SDKs to pick this out and update the priority in workflow info?

Yes, most likely.

Hrmm. Workflow info has traditionally been considered immutable for the life of the workflow in some SDKs. This is because people store it, clone it, mock it in tests, etc. People often have expectations of workflow start info as being immutable in some SDKs (but not all).

We can work around this as needed, but it is a bit rough to turn immutable values into mutable in backwards-compatible ways.

Huh; is that different from versioning overrides? They already update the WorkflowExecutionInfo: https://github.com/temporalio/temporal/blob/44017dcc4949143cb65701694938deb898bcef72/service/history/api/updateworkflowoptions/api.go#L143 which updates https://github.com/temporalio/api/blob/master/temporal/api/workflow/v1/message.proto#L93 AFAICT

Search attributes and memo are in workflow info too and they are very mutable (and that's been true forever, at least for SAs)

Yeah, I don't think it's a big problem. For those things we have provided them as methods rather than field access, but unfortunately this is already a field IIRC is really the only issue.

Either way, fine.

Huh; is that different from versioning overrides?

I believe workflow deployment information in SDKs is provided via a getter for this exact reason and not put on workflow info. But I would have to check, as I don't even see worker deployment info available to workflows in all SDKs at this time.

Search attributes and memo are in workflow info too and they are very mutable (and that's been true forever, at least for SAs)

Right, they are mutable collections on the info in many SDKs, we don't replace anything on the info itself.

but unfortunately this is already a field IIRC is really the only issue

Right, but we can keep the field representation as "original" and add getters if we want, or we can just hack this in. But it is a hack for some languages. Because when we say, e.g. in Python, Info is a frozen dataclass, the cheating it requires is a bit gross (as it should be, we have set an expectation on how deterministically mutable one can expect these values when they see we mark the info as immutable).

In Java and .NET and Ruby it will also a be a bad hack. If the Rust SDK existed, we just wouldn't be able to do this at all, but we call it "initial information" there maybe to clarify.

Doesn't affect anything in this API/PR though, just bringing up since SDK discussion started here.

dnr

I'm good with this for the api, assuming we duplicate the priority in both places for activity info/options

dnr · 2025-11-21T00:44:08Z

temporal/api/history/v1/message.proto

    string attached_request_id = 3;
    // Completion callbacks attached to the running workflow execution.
    repeated temporal.api.common.v1.Callback attached_completion_callbacks = 4;
+    temporal.api.common.v1.Priority priority = 5;


Do we have to change SDKs to pick this out and update the priority in workflow info?

dnr · 2025-11-21T00:55:31Z

temporal/api/activity/v1/message.proto

+    // Overrides the activity's priority sent by the SDK.
+    temporal.api.common.v1.Priority priority = 7;


Looking again with a fresh perspective: it seems like anything other than (a) (duplicate in both places, maybe deprecate the old one) is inevitably going to confuse someone badly, and I would just go with that. PendingActivityInfo is always understood in the context of a workflow, right? So anyone who sees a PendingActivityInfo can compute the effective Priority. (Standalone activities have no workflow to inherit from so that's fine too.) If we want to expose effective priority to avoid duplicating that logic, let's do it in a new field.

(Re-reading comments, that basically matches what I said on Aug 4, which is good)

Sushisource

Please open an item in SDK features repo for us to deal with this new field after this is merged

## What changed? Allow updating priority of Workflow and Activity. Based on - temporalio/api#610 - #8103 ## Why? Users want to change the priority after starting the workflow/activity. ## How did you test it? - [ ] built - [ ] run locally and tested manually - [ ] covered by existing tests - [x] added new unit test(s) - [x] added new functional test(s)

stephanos force-pushed the update-prio branch 2 times, most recently from 626b76b to ab563c6 Compare July 30, 2025 20:06

stephanos commented Jul 30, 2025

View reviewed changes

stephanos marked this pull request as ready for review July 30, 2025 20:07

stephanos requested review from a team as code owners July 30, 2025 20:07

stephanos requested a review from dnr July 30, 2025 20:07

Sushisource reviewed Aug 1, 2025

View reviewed changes

dnr reviewed Aug 4, 2025

View reviewed changes

stephanos force-pushed the update-prio branch from e7e3221 to 0c4d68b Compare August 21, 2025 22:10

stephanos requested a review from dnr August 22, 2025 15:43

stephanos force-pushed the update-prio branch from 70eb435 to aa4b89c Compare August 22, 2025 15:45

stephanos force-pushed the update-prio branch from aa4b89c to 1d81997 Compare October 1, 2025 21:08

stephanos mentioned this pull request Oct 2, 2025

Update Workflow/Activity priority temporalio/temporal#8396

Merged

5 tasks

dnr approved these changes Nov 21, 2025

View reviewed changes

Sushisource approved these changes Nov 25, 2025

View reviewed changes

stephanos added 4 commits November 28, 2025 10:13

Update Workflow and Activity Priority

b74e23b

add comment

237579d

update comment

a0d93d5

update comment

a375f21

stephanos force-pushed the update-prio branch from 9e516b2 to a375f21 Compare November 28, 2025 18:14

stephanos enabled auto-merge (squash) November 28, 2025 18:15

stephanos merged commit e39bdc7 into temporalio:master Nov 28, 2025
4 checks passed

stephanos deleted the update-prio branch November 28, 2025 22:17



		// Priority metadata
		// Deprecated. Use `activity_options.priority`.

		// Overrides the activity's priority sent by the SDK.
		temporal.api.common.v1.Priority priority = 7;

Update Workflow and Activity Priority #610

Update Workflow and Activity Priority #610

Uh oh!

Conversation

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

Uh oh!

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!

stephanos Aug 1, 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!

stephanos Oct 9, 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!

cretz Nov 21, 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!

stephanos Nov 21, 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 Nov 25, 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!

dnr 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!

Sushisource left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stephanos commented Jul 8, 2025 •

edited

Loading

stephanos Aug 1, 2025 •

edited

Loading

stephanos Oct 9, 2025 •

edited

Loading

cretz Nov 21, 2025 •

edited

Loading

stephanos Nov 21, 2025 •

edited

Loading

cretz Nov 25, 2025 •

edited

Loading