[Deployments] Sync workflow before activity tqs and batch updates #7003
Conversation
| }) | ||
| } | ||
| if workflow.GetVersion(ctx, "syncToTaskQueues", workflow.DefaultVersion, 0) == workflow.DefaultVersion { | ||
| err = d.syncToTaskQueues1(ctx) |
There was a problem hiding this comment.
Nit: should we call these syncToTaskQueues0 and syncToTaskQueues1 to match the version?
| // Sync and wait for workflow task queues, then activity task queues, to ensure that | ||
| // workflows don't bounce between deployments due to activity/wft starts. | ||
| enumspb.TASK_QUEUE_TYPE_WORKFLOW, | ||
| enumspb.TASK_QUEUE_TYPE_ACTIVITY, |
There was a problem hiding this comment.
We want to support Nexus tasks too. they should be synced with activities I believe.
| byType := d.State.TaskQueueFamilies[tqName].TaskQueues | ||
| for _, tqTypeInt := range workflow.DeterministicKeys(byType) { | ||
| tqType := enumspb.TaskQueueType(tqTypeInt) | ||
| if doWorkflow == (tqType == enumspb.TASK_QUEUE_TYPE_WORKFLOW) { |
There was a problem hiding this comment.
I thought about this more. I don't think anymore that syncing WF type first will help us because it will make another race condition which might be even worse: wf moves to the new build via a WFT and now it schedules an activity but the activity goes to the previous build because the activity TQ user data is not yet updated.
I think it's best to keep things simple and sync all types in a single type. That also reduces the number of user data writes. For now, we can accept the back-and-forth between versions for unpinned wfs when the sync is not propagated to all partitions. That should be fine though because it happens all the time in unversioned rn. Later I think we can improve this by making the sync 2-phased: prepare then sync.
|
It seems this wont land before we clone to entity workflows with new names. Let's close it but once the new wfs are added, add the batching part to the new wfs only. The workflow vs activity separation is not needed anymore. |
Sounds good to me |
What changed?
Why?
How did you test it?
existing tests
Potential risks
the workflow logic change is guarded with GetVersion call