Versioning History <-> Matching protos #6874
Conversation
| @@ -120,10 +121,13 @@ message AddWorkflowTaskRequest { | |||
| // for TaskVersionDirective, which is unversioned.) | |||
| temporal.server.api.taskqueue.v1.TaskVersionDirective version_directive = 10; | |||
| temporal.server.api.taskqueue.v1.TaskForwardInfo forward_info = 11; | |||
| // Presence of this field means the workflow is versioned. | |||
| temporal.server.api.deployment.v1.WorkflowVersioningInfo workflow_versioning_info = 12; | |||
There was a problem hiding this comment.
| temporal.server.api.deployment.v1.WorkflowVersioningInfo workflow_versioning_info = 12; | |
| temporal.server.api.deployment.v1.TargetWorkerDeployment target_worker_deployment = 12; |
There was a problem hiding this comment.
Rationale: when History adds task/query to matching it expect it to be executed on this pinned or unpinned worker deployment. I would also remove word "worker" but then it can be confused with "deployment we are about to rollout". If you think it is not a case, you can call it just target_deployment.
There was a problem hiding this comment.
pinned or unpinned worker deployment
deployment is not pinned or unpinned itself, workflow could be pinned or unpinned.
The idea is not that history gives matching the "target" deployment. It just gives matching the workflow versioning info and matching decides how to route based on that info. "Target deployment" does not fit well with unpinned because when matching does the redirect the deployment sent here would be the source of the redirect which will be confusing because "target" is almost antonym of "source".
There was a problem hiding this comment.
Conclusion: add the two fields to TaskVersionDirective
| @@ -74,4 +75,20 @@ message DescribeResponse { | |||
| message DeploymentWorkflowMemo { | |||
| google.protobuf.Timestamp create_time = 1; | |||
| bool is_current_deployment = 2; | |||
There was a problem hiding this comment.
isn't this effective now? I might confuse them though.
There was a problem hiding this comment.
no this is about deployment themselves, not the effective deployment of a particular execution.
| // Whether the workflow deployment that Matching received when the task was scheduled contains | ||
| // the task's task queue. Used by History to determine if the task redirect should affect the | ||
| // workflow. | ||
| temporal.api.deployment.v1.Deployment schedule_time_workflow_deployment_contains_task_queue = 2; |
There was a problem hiding this comment.
Conclusion: add fields to the record start requests directly, if needed.
…ernal-protos # Conflicts: # api/deployment/v1/message.pb.go # api/historyservice/v1/request_response.pb.go # proto/internal/temporal/server/api/deployment/v1/message.proto
## What changed? <!-- Describe what has changed in this PR --> Add versioning v3 fields to the internal APIs between History and Matching so they can send needed info when tasks are scheduled or started. ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
## What changed? <!-- Describe what has changed in this PR --> Add versioning v3 fields to the internal APIs between History and Matching so they can send needed info when tasks are scheduled or started. ## Why? <!-- Tell your future self why have you made these changes --> ## How did you test it? <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> ## Potential risks <!-- Assuming the worst case, what can be broken when deploying this change to production? --> ## Documentation <!-- Have you made sure this change doesn't falsify anything currently stated in `docs/`? If significant new behavior is added, have you described that in `docs/`? --> ## Is hotfix candidate? <!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
What changed?
Add versioning v3 fields to the internal APIs between History and Matching so they can send needed info when tasks are scheduled or started.
Why?
How did you test it?
Potential risks
Documentation
Is hotfix candidate?