-
Notifications
You must be signed in to change notification settings - Fork 1.8k
WCP 7/X: Change failure cause and expose workflow task completion size limit #11377
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -383,7 +383,7 @@ func (handler *WorkflowTaskCompletedHandler) Invoke( | |
| if paginationOverflow { | ||
| // Per-workflow completion buffer overflowed: terminate the workflow | ||
| wtFailedCause = newWorkflowTaskFailedCause( | ||
| enumspb.WORKFLOW_TASK_FAILED_CAUSE_PAYLOADS_TOO_LARGE, | ||
| enumspb.WORKFLOW_TASK_FAILED_CAUSE_REQUEST_TOO_LARGE, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just confirm my understanding that SDK will consume the new enum compatibly
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Discussed it offline. Not sure what you mean by "compatibly" here, can talk more if something is unclear. |
||
| serviceerror.NewInvalidArgument( | ||
| "workflow task completion buffer size exceeds the per-workflow limit"), | ||
| true) | ||
|
|
||
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.
When pagination is enabled and the final page carries commands, this reports the buffer limit as the maximum total workflow-task-completion size, but
AppendTaskCompletionPageenforces it only against intermediate-page commands (service/history/workflow/context.go:238-244);GetMergedTaskCompletionPagesadds the final-page size only to a metric (context.go:346-350), and non-paginated completions bypass the limit entirely. Consequently, a server configured with a small limit can accept completions much larger than the value advertised to SDKs, so the new field cannot reliably guide their paging decisions; either enforce this value against the complete logical request or expose a limit whose documented semantics match the buffer-only check.Useful? React with 👍 / 👎.
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.
I'll think about this. But out of scope for this change.