Workflow Task stamp by stephanos · Pull Request #8103 · temporalio/temporal

stephanos · 2025-07-28T18:11:53Z

What changed?

added "stamp" field to both workflow execution and transfer/timer workflow task.
bumping stamp up when rescheduling a workflow task
making sure stamp is current when dequeuing workflow task
making sure stamp is current when recoding workflow start

Why?

Activity tasks already have the "stamp" mechanism. This is the same idea for workflow tasks.

It essentially provides a unified approach to invalidate activity and workflow tasks when rescheduling them.

How did you test it?

I intend to write more tests for when I'm implementing the update priority feature. It's much easier to setup and test than versioning overrides (which are the only way right now to trigger a stamp increase).

stephanos · 2025-07-28T18:12:21Z

@@ -60,7 +60,3 @@ func (r *ActivityRetryTimerTask) GetType() enumsspb.TaskType {
 func (r *ActivityRetryTimerTask) GetStamp() int32 {
 	return r.Stamp
 }
-
-func (r *ActivityRetryTimerTask) SetStamp(stamp int32) {


SetStamp functions aren't used anywhere.

stephanos · 2025-07-29T17:23:02Z

@@ -5117,8 +5121,6 @@ func (s *mutableStateSuite) TestHasRequestID() {
 }

 func (s *mutableStateSuite) TestHasRequestID_StateConsistency() {
-	s.SetupTest()


Not needed.

stephanos · 2025-07-29T17:50:21Z

-		if ms.HasPendingWorkflowTask() && !ms.HasStartedWorkflowTask() &&
-			// Speculative WFT is directly (without transfer task) added to matching when scheduled.
-			// It is protected by timeout on both normal and sticky task queues.
-			// If there is no poller for previous deployment, it will time out,
-			// and will be rescheduled as normal WFT.
-			ms.GetPendingWorkflowTask().Type != enumsspb.WORKFLOW_TASK_TYPE_SPECULATIVE {
-			// Sticky queue was just cleared, so the following call only generates a WorkflowTask, not a WorkflowTaskTimeoutTask.
-			err := ms.taskGenerator.GenerateScheduleWorkflowTaskTasks(ms.GetPendingWorkflowTask().ScheduledEventID)
-			if err != nil {
-				return err
-			}
-		}


Migrated into reschedulePendingWorkflowTask.

stephanos · 2025-07-31T01:03:22Z

@@ -17,6 +17,7 @@ type (
 		TaskQueue           string
 		ScheduledEventID    int64
 		Version             int64
+		Stamp               int32


Is my thinking right that only the workflow transfer and timeout task need the stamp?

For workflow pause workstream we might need it in UserTimerTask & StartChildExecutionTask as well. But we are not ready for it yet. So we can add it in the future if needed.

stephanos · 2025-07-31T01:03:49Z

@@ -1056,7 +1059,6 @@ func (s *mutableStateSuite) TestOverride_RedirectFails() {
 }

 func (s *mutableStateSuite) TestOverride_BaseDeploymentUpdatedOnCompletion() {
-	s.T().Skip("TODO (Shahab)")


This test works fine AFAICT.

stephanos · 2025-07-31T03:08:26Z

@@ -74,6 +73,10 @@ func Invoke(
 				//  - Speculative WFT is lost (ScheduleToStart timeout for speculative WFT will recreate it).
 				return nil, serviceerror.NewNotFound("Workflow task not found.")
 			}
+			if workflowTask.Stamp != mutableState.GetExecutionInfo().GetWorkflowTaskStamp() {
+				// This happens when the workflow task was rescheduled.
+				return nil, serviceerrors.NewObsoleteMatchingTask("Workflow task stamp mismatch")


Only place outside of task processing internals where we invalidate a workflow task with an outdated stamp now.

My thinking here is that when deploying a new version, all in-flight tasks and running workflows will have the default stamp, ie zero, and match. Only when the tasks are re-scheduled would they not match.

stephanos · 2025-10-02T03:55:43Z

@gow Alright; check out the 2nd commit I added. There's also a follow-up PR (will be reviewed by matching crew) where I actually use the stamp and have a functional test for it: #8396 (see tests/priority_fairness_test.go).

yycptt · 2025-10-03T23:09:48Z

+    // Stamp value from when the workflow task was scheduled. Used to validate the task is still relevant.
+    int32 stamp = 11;


ideally should have attempt wherever there's stamp. But we can save that for later :)

yycptt · 2025-10-03T23:20:13Z

 			EventID:             workflowTask.ScheduledEventID,
 			ScheduleAttempt:     workflowTask.Attempt,
 			Version:             workflowTask.Version,
+			Stamp:               r.mutableState.GetExecutionInfo().GetWorkflowTaskStamp(), // must use current stamp!


oh maybe provide some more context here. Guess I am still confused on why workflow.Stamp would be wrong here?

Alright; I took another look and when I wrote this I didn't realize that GetWorkflowTaskByID is actually just returning a struct with the current workflow task's info. I thought it returned the currently scheduled workflow task (I didn't know we don't store that). So I've removed the special case here now!

yycptt · 2025-10-03T23:23:35Z

+	// Reset the attempt; forcing a non-transient workflow task to be scheduled.
+	ms.executionInfo.Attempt = 1


we should only do this when Stamp is incremented?

You're right 👍

yycptt · 2025-10-03T23:25:08Z

We need to make sure the stamp field is replicated properly in state-based replication as well. Please update the syncExecutionInfo() method in this file and have the Stamp field updated.

Oh; that's easy to miss if you don't know where to look! Have we considered some kind of test that verifies a field is either replicated or explicitly ignored?

I'm not sure if I got the test right.

yeah, I think CGS team is working on some general testing strategy to assert the states are the same on both sides.

gow · 2025-10-04T00:45:41Z

-) error {
-	previousEffectiveDeployment := ms.GetEffectiveDeployment()
-	previousEffectiveVersioningBehavior := ms.GetEffectiveVersioningBehavior()
+) (requestReschedulePendingWorkflowTask bool, err error) {


nit: Prefer avoiding named return values in large and complex functions. It hurts readability since return statements are naked and hard to know what values are being returned.

Hm, I 100% agree with that except here because without it, it just says bool and you'll have to scan the whole body to find out what the bool is. Alternatives I can think of are (1) add comment (can get out of date more easily) or (2) adding a var requestReschedulePendingWorkflowTask bool in the body right at the top. WDYT?

Using var now, looks fine 👍

gow · 2025-10-04T00:48:33Z

-			return err
-		}
+	// Re-scheduling pending workflow and activity tasks.
+	err = ms.reschedulePendingWorkflowTask(false)


This seems like a case where we want to invalidate existing tasks? i.e ms.reschedulePendingWorkflowTask(true)?

So the reason I don't is I cut scope from this PR. The previous code didn't invalidate pending workflow tasks, so there's no behavior change. The recordworkflowtaskstarted API has a dedicated check for buildId.

Why did I declare it out of scope? Yichao asked me how this works with replication and I couldn't tell you since I don't understand the versioning API well enough. So in order to get this merged, I declared it out of scope.

Those versioning rescheduling logic is not working with state-based replication yet. That's something replication crew is aware of and will be fixing them soon. cc @yux0 @xwduan

gow

Looks good to me. Let's wait for @yycptt 's stamp as well.

yycptt · 2025-10-24T23:33:07Z

yeah, I think CGS team is working on some general testing strategy to assert the states are the same on both sides.

## 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 commented Jul 28, 2025

View reviewed changes

stephanos force-pushed the workflow-task-stamp branch 9 times, most recently from 5f2fc0c to 057109e Compare July 29, 2025 17:21

stephanos commented Jul 29, 2025

View reviewed changes

stephanos force-pushed the workflow-task-stamp branch 2 times, most recently from d317e6f to 23504cf Compare July 29, 2025 17:39

stephanos commented Jul 29, 2025

View reviewed changes

stephanos force-pushed the workflow-task-stamp branch from 23504cf to 8a6f0dd Compare July 29, 2025 17:52

stephanos commented Jul 31, 2025

View reviewed changes

stephanos force-pushed the workflow-task-stamp branch 2 times, most recently from 878355d to ddbe4ca Compare July 31, 2025 03:02

stephanos marked this pull request as ready for review July 31, 2025 03:02

stephanos requested a review from a team as a code owner July 31, 2025 03:02

stephanos commented Jul 31, 2025

View reviewed changes

stephanos force-pushed the workflow-task-stamp branch from 1a5d6bf to f5b73ab Compare August 6, 2025 18:30

stephanos force-pushed the workflow-task-stamp branch 7 times, most recently from 5b6dc75 to 0bec537 Compare September 18, 2025 17:07

stephanos requested a review from gow October 2, 2025 03:54

stephanos mentioned this pull request Oct 2, 2025

Update Workflow/Activity priority #8396

Merged

5 tasks

yycptt reviewed Oct 3, 2025

View reviewed changes

gow reviewed Oct 4, 2025

View reviewed changes

stephanos requested a review from a team as a code owner October 7, 2025 18:19

stephanos force-pushed the workflow-task-stamp branch 2 times, most recently from 1ec84ce to fc967bf Compare October 7, 2025 18:51

stephanos requested review from gow and yycptt October 7, 2025 21:13

gow approved these changes Oct 10, 2025

View reviewed changes

yycptt approved these changes Oct 24, 2025

View reviewed changes

stephanos added 10 commits October 29, 2025 17:10

Workflow Task stamp

c5035c0

Workflow Stamp from TaskInfo

f8acfda

move Attempt reset into if

3bb1388

use var instead

b5968c1

add stamp to IsWorkflowTaskValidRequest

d796be9

don't clear cache on ErrStaleReference

5d7ef9e

go fmt

e3371b6

sync stamp

6caaa38

simplify

291d31d

fix unit tests

741f109

stephanos force-pushed the workflow-task-stamp branch from 826b806 to 741f109 Compare October 30, 2025 00:10

stephanos added 4 commits October 29, 2025 18:22

fix linter

81c241e

fix linter

6810298

fix linter

16fa16a

fix linter

1ff6dbf

stephanos merged commit 1e3d3af into temporalio:main Oct 30, 2025
57 checks passed

stephanos deleted the workflow-task-stamp branch October 30, 2025 03:27

		// Stamp value from when the workflow task was scheduled. Used to validate the task is still relevant.
		int32 stamp = 11;

		// Reset the attempt; forcing a non-transient workflow task to be scheduled.
		ms.executionInfo.Attempt = 1

Uh oh!

Conversation

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

Uh oh!

What changed?

Why?

How did you test it?

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!

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

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!

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!

gow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stephanos commented Jul 28, 2025 •

edited

Loading

stephanos commented Oct 2, 2025 •

edited

Loading