You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Replace SuggestContinueAsNew + SuggestContinueAsNewReasonTargetVersionChanged with new TargetVersionChanged flag (#709)
_**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._
<!-- Describe what has changed in this PR -->
- Replace SuggestContinueAsNew +
SuggestContinueAsNewReasonTargetVersionChanged with new
TargetVersionChanged flag
- Add new `target_worker_deployment_version_changed` flag to be sent in
WorkflowTaskStartedEvent and exposed next to `continue-as-new-suggested`
in the workflow info.
<!-- Tell your future self why have you made these changes -->
**Why?**
Setting `SuggestContinueAsNew=true` for Pinned workflows whenever their
is a new Target Version available for that workflow causes Pinned
workflows to hit that condition much more frequently than they expect.
Users who are currently doing: `if workflow_info.suggestContinueAsNew{
do continue-as-new }` in their Pinned workflow code would need to change
that code to protect themselves from running into an infinite-CaN-loop,
because the default CaN behavior for a Pinned workflow is to stay
Pinned.
We should not force users to protect themselves from such a situation.
Because upgrading on continue-as-new is opt-in, receiving the suggestion
to continue-as-new-onto-new-target-version should be opt-in as well. If
people are forced to check the new `suggest-continue-as-new-reasons`
field to "opt out," that is unsafe, because inevitably some people will
forget to do so or misunderstand, and then get hit by this unexpected
footgun.
Much safer and still ergonomical to let upgrade-on-can be opt-in on both
fronts, as proposed here. With this change, the people who are currently
doing `if workflow_info.suggestContinueAsNew{ do continue-as-new }`
won't see any change in semantics, regardless of their versioning
behavior.
People who consciously know that they want to do upgrade-on-can /
Trampolining will have to change their CaN options anyway, so it's easy
enough to teach them to pay attention to this new
`TargetWorkerDeploymentVersionChanged` flag.
<!-- Are there any breaking changes on binary or code level? -->
**I am proposing to completely remove the "new target version" reason
from the can-suggested reasons list. Some SDK PRs use it, and they
should stop. We could also simply deprecate the reason and just tell
people that it's not used anymore, but I'd prefer to flat out delete it.
Server is already patched to not send that flag by default.**
<!-- If this breaks the Server, please provide the Server PR to merge
right after this PR was merged. -->
**Server PR**
temporalio/temporal#9239
---------
Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io>
"description": "SuggestContinueAsNewReason specifies why SuggestContinueAsNew is true.\n\n - SUGGEST_CONTINUE_AS_NEW_REASON_HISTORY_SIZE_TOO_LARGE: Workflow History size is getting too large.\n - SUGGEST_CONTINUE_AS_NEW_REASON_TOO_MANY_HISTORY_EVENTS: Workflow History event count is getting too large.\n - SUGGEST_CONTINUE_AS_NEW_REASON_TOO_MANY_UPDATES: Workflow's count of completed plus in-flight updates is too large.\n - SUGGEST_CONTINUE_AS_NEW_REASON_TARGET_WORKER_DEPLOYMENT_VERSION_CHANGED: Workflow's Target Worker Deployment Version is different from its\nCurrent Version and the workflow is versioned."
16185
+
"description": "SuggestContinueAsNewReason specifies why SuggestContinueAsNew is true.\n\n - SUGGEST_CONTINUE_AS_NEW_REASON_HISTORY_SIZE_TOO_LARGE: Workflow History size is getting too large.\n - SUGGEST_CONTINUE_AS_NEW_REASON_TOO_MANY_HISTORY_EVENTS: Workflow History event count is getting too large.\n - SUGGEST_CONTINUE_AS_NEW_REASON_TOO_MANY_UPDATES: Workflow's count of completed plus in-flight updates is too large."
16187
16186
},
16188
16187
"v1TaskIdBlock": {
16189
16188
"type": "object",
@@ -18444,6 +18443,10 @@
18444
18443
},
18445
18444
"description": "The reason(s) that suggest_continue_as_new is true, if it is.\nUnset if suggest_continue_as_new is false."
18446
18445
},
18446
+
"targetWorkerDeploymentVersionChanged": {
18447
+
"type": "boolean",
18448
+
"description": "True if Workflow's Target Worker Deployment Version is different from its Pinned Version and\nthe workflow is Pinned.\nExperimental."
0 commit comments