Skip to content

Conversation

dnr
Copy link
Contributor

@dnr dnr commented Mar 18, 2025

What changed?
Add initial protos for priority, enough to attach simple priority metadata to workflows and activities.

See #513 for the full future plans, only the first field (priority_key) is included so far.

Why?
Control task ordering.

Server PR
Implementation on priority branch: temporalio/temporal@main...priority

dnr and others added 4 commits January 17, 2025 17:35
**What changed?**
Add initial protos for priority, enough to attach priority metadata to
workflows and activities.

Later, we'll add APIs to update priority of existing workflows and
activities, to override values in task queue configuration, and
observability APIs.

This has the full planned `Priority` message to provide context for
reviewers, even though the implementation will come slowly over time.
Specifically, in the first phase only the `priority` field will be
meaningful. I'll comment out/delete the rest before merging and add them
back later.

**Why?**
Control task ordering.

---------

Co-authored-by: Spencer Judge <sjudge@hey.com>
Co-authored-by: Yuri <ychebotarev@gmail.com>
Co-authored-by: Rodrigo Zhou <2068124+rodrigozhou@users.noreply.github.com>
Co-authored-by: Carly de Frondeville <carly.defrondeville@temporal.io>
**What changed?**
Remove future planned fields from Priority message for now.

**Why?**
Prepare for merging to main. Only priority_key is implemented so far.
@dnr dnr requested review from a team as code owners March 18, 2025 18:23
dnr and others added 2 commits March 18, 2025 12:28
Co-authored-by: Spencer Judge <spencer@temporal.io>
Comment on lines 283 to 285
// If priority is not present (or zero), then the effective priority will be
// the default priority, which is is calculated by (min+max)/2. With the
// default max of 5, and min of 1, that comes out to 3.
Copy link
Member

Choose a reason for hiding this comment

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

If priority is not present (or zero), then the effective priority will be the default priority, which is is calculated by (min+max)/2. With the default max of 5, and min of 1, that comes out to 3.

This is a bit in conflict (or at least confusing) with the comment above the message which states:

For all fields, the field not present or equal to zero/empty string means to inherit the value from the calling workflow, or if there is no calling workflow, then use the default value.

Should the first quoted statement change "If priority is not present (or zero)" to "If priority is not present (or zero) and there is no calling workflow"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see the confusion: the above comment is talking about how to merge these messages if there's one on the workflow and also an activity call (or child wf), the one here is talking from the perspective of a post-merged message and saying what happens if it's still zero. I'll try to reword.

@dnr dnr merged commit 3400eb6 into master Mar 19, 2025
4 checks passed
@dnr dnr deleted the priority branch March 19, 2025 02:57
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.

4 participants