Skip to content
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.

Sign up for GitHub

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

Add OnConflictOptions to StartWorkflowExecutionRequest #510

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+79 −0
Open

Add OnConflictOptions to StartWorkflowExecutionRequest #510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions openapi/openapiv2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6552,6 +6552,10 @@
"versioningOverride": {
"$ref": "#/definitions/v1VersioningOverride",
"description": "If set, takes precedence over the Versioning Behavior sent by the SDK on Workflow Task completion.\nTo unset the override after the workflow is running, use UpdateWorkflowExecutionOptions."
},
"onConflictOptions": {
"$ref": "#/definitions/v1OnConflictOptions",
"description": "It defines behaviors to be executed when the WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING\nconflict policy is used. If set to non-nil object, it will attach the RequestID to the\nexisting running workflow (for deduping), and perform actions specified in the object.\nIf not set (ie., nil value), it won't do anything to the existing running workflow."
}
}
},
Expand Down Expand Up @@ -9975,6 +9979,18 @@
},
"description": "Nexus operation timed out."
},
"v1OnConflictOptions": {
"type": "object",
"properties": {
"attachCompletionCallbacks": {
"type": "boolean"
},
"attachLinks": {
"type": "boolean"
}
},
"description": "When StartWorkflowExecution uses the WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING conflict policy and\nthere is already an existing running workflow, OnConflictOptions defines actions to be taken on\nthe existing running workflow."
},
"v1Outcome": {
"type": "object",
"properties": {
Expand Down Expand Up @@ -12004,6 +12020,10 @@
"versioningOverride": {
"$ref": "#/definitions/v1VersioningOverride",
"description": "If set, takes precedence over the Versioning Behavior sent by the SDK on Workflow Task completion.\nTo unset the override after the workflow is running, use UpdateWorkflowExecutionOptions."
},
"onConflictOptions": {
"$ref": "#/definitions/v1OnConflictOptions",
"description": "It defines behaviors to be executed when the WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING\nconflict policy is used. If set to non-nil object, it will attach the RequestID to the\nexisting running workflow (for deduping), and perform actions specified in the object.\nIf not set (ie., nil value), it won't do anything to the existing running workflow."
}
}
},
Expand Down Expand Up @@ -13074,6 +13094,18 @@
"versioningOverride": {
"$ref": "#/definitions/v1VersioningOverride",
"description": "Versioning override in the mutable state after event has been applied."
},
"attachedRequestId": {
"type": "string",
"description": "Attached request ID to the running workflow execution so that additional requests with same\nrequest ID will be deduped."
},
"attachedCompletionCallbacks": {
"type": "array",
"items": {
"type": "object",
"$ref": "#/definitions/v1Callback"
},
"description": "Attached completion callbacks to the running workflow execution."
}
}
},
Expand Down
29 changes: 29 additions & 0 deletions openapi/openapiv3.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7443,6 +7443,17 @@ components:
type: string
description: The request ID allocated at schedule time.
description: Nexus operation timed out.
OnConflictOptions:
type: object
properties:
attachCompletionCallbacks:
type: boolean
attachLinks:
type: boolean
description: |-
When StartWorkflowExecution uses the WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING conflict policy and
there is already an existing running workflow, OnConflictOptions defines actions to be taken on
the existing running workflow.
Outcome:
type: object
properties:
Expand Down Expand Up @@ -9379,6 +9390,14 @@ components:
description: |-
If set, takes precedence over the Versioning Behavior sent by the SDK on Workflow Task completion.
To unset the override after the workflow is running, use UpdateWorkflowExecutionOptions.
onConflictOptions:
allOf:
- $ref: '#/components/schemas/OnConflictOptions'
description: |-
It defines behaviors to be executed when the WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING
conflict policy is used. If set to non-nil object, it will attach the RequestID to the
existing running workflow (for deduping), and perform actions specified in the object.
If not set (ie., nil value), it won't do anything to the existing running workflow.
StartWorkflowExecutionResponse:
type: object
properties:
Expand Down Expand Up @@ -10570,6 +10589,16 @@ components:
allOf:
- $ref: '#/components/schemas/VersioningOverride'
description: Versioning override in the mutable state after event has been applied.
attachedRequestId:
type: string
description: |-
Attached request ID to the running workflow execution so that additional requests with same
request ID will be deduped.
attachedCompletionCallbacks:
type: array
items:
$ref: '#/components/schemas/Callback'
description: Attached completion callbacks to the running workflow execution.
WorkflowExecutionSignaledEventAttributes:
type: object
properties:
Expand Down
5 changes: 5 additions & 0 deletions temporal/api/history/v1/message.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,11 @@ message ChildWorkflowExecutionTerminatedEventAttributes {
message WorkflowExecutionOptionsUpdatedEventAttributes {
// Versioning override in the mutable state after event has been applied.
temporal.api.workflow.v1.VersioningOverride versioning_override = 1;
// Attached request ID to the running workflow execution so that additional requests with same
// request ID will be deduped.
string attached_request_id = 2;
// Attached completion callbacks to the running workflow execution.
repeated temporal.api.common.v1.Callback attached_completion_callbacks = 3;
}

// Not used anywhere. Use case is replaced by WorkflowExecutionOptionsUpdatedEventAttributes
Expand Down
8 changes: 8 additions & 0 deletions temporal/api/workflow/v1/message.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,3 +440,11 @@ message VersioningOverride {
// Identifies the worker deployment to pin the workflow to.
temporal.api.deployment.v1.Deployment deployment = 2;
}

// When StartWorkflowExecution uses the WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING conflict policy and
// there is already an existing running workflow, OnConflictOptions defines actions to be taken on
// the existing running workflow.
Copy link
Member

Choose a reason for hiding this comment

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

I'd mention that we plan to add options here as needed.

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'm adding explicitly the behaviors we want to implement at first already, so I think no need to update comment.

message OnConflictOptions {
Copy link
Member

Choose a reason for hiding this comment

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

Will this message be used anywhere else except start workflow?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe in schedules?

Copy link
Member

Choose a reason for hiding this comment

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

There is no ID conflict policy in schedules (by intention as I understand it due to how schedules set IDs)

bool attach_completion_callbacks = 1;
bool attach_links = 2;
Comment on lines +448 to +449
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
bool attach_completion_callbacks = 1;
bool attach_links = 2;
bool append_completion_callbacks = 1;
bool append_links = 2;

I think this may be clearer terminology (assuming it is actually "appending")

Copy link
Member

Choose a reason for hiding this comment

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

This works for me although I prefer the term attach over append but don't have a strong opinion on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For callbacks, "append" makes sense since we adding to the workflow execution.
But for links, this is set in the history event, it's not "appending" an existing list/object.
I'm ok with changing for callbacks, but I'd leave "attach" for links.

Copy link
Member

Choose a reason for hiding this comment

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

I think what each of these do to existing values may need to be documented here because it is clearly confusing. What does "attach" mean for links? If links are set in the history event and immutable, then where in history are these new ones set? Or are there now two kinds of links, start links and post-start links, and they manifest in different ways?

Copy link
Member

Choose a reason for hiding this comment

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

Agree we should document these options.

The link would essentially be linked to the new event but conceptually they're considered attached to the workflow.

flowchart LR
    CallerWorkflowA --> HandlerWorkflow
    CallerWorkflowB --> HandlerWorkflow
Loading

The callback is added to the list of completion callbacks.

Copy link
Member

Choose a reason for hiding this comment

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

Which new event? I am under the impression that a use-existing conflict policy will not make any new events if the workflow already exists.

}
5 changes: 5 additions & 0 deletions temporal/api/workflowservice/v1/request_response.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,11 @@ message StartWorkflowExecutionRequest {
// If set, takes precedence over the Versioning Behavior sent by the SDK on Workflow Task completion.
// To unset the override after the workflow is running, use UpdateWorkflowExecutionOptions.
temporal.api.workflow.v1.VersioningOverride versioning_override = 25;
// It defines behaviors to be executed when the WORKFLOW_ID_CONFLICT_POLICY_USE_EXISTING
// conflict policy is used. If set to non-nil object, it will attach the RequestID to the
// existing running workflow (for deduping), and perform actions specified in the object.
// If not set (ie., nil value), it won't do anything to the existing running workflow.
temporal.api.workflow.v1.OnConflictOptions on_conflict_options = 26;
Copy link
Member

Choose a reason for hiding this comment

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

Is this needed for signal-with-start?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so, because for signal with start we're thinking that you care more about the signal than the start and we don't plan on attaching the Nexus operation to the workflow's lifecycle.

Copy link
Member

@cretz cretz Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

But this is a general purpose concept not specific to Nexus. In general everywhere a conflict policy is applied it'd make sense to apply conflict options IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the case of signal with start, the goal is to signal the workflow if it's already running, not update it.
Supporting OnConflictOptions in this case would mean that the behavior of signal with start would be to update and signal the workflow? Would this make sense?

Copy link
Member

Choose a reason for hiding this comment

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

I think the general idea of this being paired with a conflict policy makes sense for consistency reasons. I think your question could be rephrased as "does conflict policy make sense on signal with start", but since it was decided it does however that was decided, I think these options should go alongside them.

Copy link
Member

Choose a reason for hiding this comment

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

I don't see a use case for attaching callbacks and links to a running workflow on signal-with-start.
We plan to attach a link on the WorkflowExecutionSignaled event (and probably on the WorkflowExecutionStarted event if the current request started a new workflow). But we do not have plans to attach callbacks on signal-with-start.
I'd rather only add fields that we plan on using.

Copy link
Member

Choose a reason for hiding this comment

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

On conflict options is about what do to on conflict. Whether a use case exists for these first couple of options I don't think affects the fact that everywhere a conflict policy is applied, its conflict options should apply.

Copy link
Member

Choose a reason for hiding this comment

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

I would still rather add this later. We don't plan on adding server support for this quite yet.

Copy link
Member

Choose a reason for hiding this comment

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

I do think conflict policy and options need to be married (arguably if we needed both at the same time they would not have been separate objects). Supporting different conflict behaviors on different start calls is confusing.

}

message StartWorkflowExecutionResponse {
Expand Down
Loading