-
Notifications
You must be signed in to change notification settings - Fork 69
add create_request_id to child initiated event #598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -665,6 +665,9 @@ message StartChildWorkflowExecutionInitiatedEventAttributes { | |||
bool inherit_build_id = 19; | |||
// Priority metadata | |||
temporal.api.common.v1.Priority priority = 20; | |||
// A unique identifier for the request to start this child workflow execution. This is used to | |||
// deduplicate requests to start a child workflow execution with the same workflow ID. | |||
string create_request_id = 21; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string create_request_id = 21; | |
string request_id = 21; |
May not need the create_
prefix, but not a big deal
@@ -665,6 +665,9 @@ message StartChildWorkflowExecutionInitiatedEventAttributes { | |||
bool inherit_build_id = 19; | |||
// Priority metadata | |||
temporal.api.common.v1.Priority priority = 20; | |||
// A unique identifier for the request to start this child workflow execution. This is used to | |||
// deduplicate requests to start a child workflow execution with the same workflow ID. | |||
string create_request_id = 21; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this event be in the map at WorkflowExecutionExtendedInfo.request_id_infos
when describing a workflow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. Child workflow's executionState.RequestIds[StartRequest.GetRequestId()] stores startEvent of the child workflow. (code)
@@ -665,6 +665,9 @@ message StartChildWorkflowExecutionInitiatedEventAttributes { | |||
bool inherit_build_id = 19; | |||
// Priority metadata | |||
temporal.api.common.v1.Priority priority = 20; | |||
// A unique identifier for the request to start this child workflow execution. This is used to | |||
// deduplicate requests to start a child workflow execution with the same workflow ID. | |||
string create_request_id = 21; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used to deduplicate requests to start a child workflow execution with the same workflow ID.
I assume the tuple of workflow run ID + event ID is not enough?
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.
Add request_id to child initiated event.
Why
For event-based replication, if request_id is not replicated to standby clusters, after failover, standby cannot dedup the start workflow request.
Breaking changes
Server PR
temporalio/temporal#7813