Repository navigation
Conversation
- Restores the behavior of `ResetWorkflowExecution` to create the reset run with its request ID, so a retry with the same `request_id` returns existing run instead of creating a new one. - The behavior is gated by the namespace dynamic config `history.enableResetWorkflowRequestIdDeduplication` (default `true`). When `false`, the reset run keeps the original start's request ID, as since temporalio#9479. Since temporalio#9479, the reset run is created with the original start's request ID. `ResetWorkflowExecution` dedupes retries by comparing the `CreateRequestId` on the mutable state with the incoming `request_id`, so the check never matches: every retried reset creates a new run and terminates the previous one. callbacks whose token has no request ID. Since temporalio#10605, with `callback.encodeInternalTokenWithEnvelope` (now default `true`), the token carries the scheduler's request ID, so completions match regardless of the reset run's request ID. - [x] built - [ ] run locally and tested manually - [x] covered by existing tests - [ ] added new unit test(s) - [x] added new functional test(s) New functional tests: * retried reset with the same request ID returns the same run * a scheduled workflow's completion is observed after reset followed by CAN and after CAN followed by reset - On by default. CHASM scheduled workflows (`history.enableCHASMSchedulerCreation=true`) started while `callback.encodeInternalTokenWithEnvelope` was false and have callback tokens without a request ID; after a reset the scheduler ignores their completion (CAN already behaves this way for them). - Reset runs no longer carry the original start's request ID, so a retry of the original start that arrives after a reset is not deduplicated. - Roll back by setting `history.enableResetWorkflowRequestIdDeduplication` to `false`. (cherry picked from commit 0b5a559)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50784f4966
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // The reset run's start event is replayed with this request ID, so it becomes the run's CreateRequestId, | ||
| // which ResetWorkflowExecution matches against the reset request ID to dedupe retries. Resets without a | ||
| // reset request ID keep the original start request ID. | ||
| requestID := cmp.Or(resetRequestID, findStartRequestID(baseWorkflow.GetMutableState().GetExecutionState())) |
There was a problem hiding this comment.
When callback.encodeInternalTokenWithEnvelope remains at its default false, using the reset request ID here removes the original scheduled-start ID from the reset run, while the legacy token carries no request ID (chasm/nexus_completion.go:25-31) and the history handler refuses current-run fallback without one (service/history/handler.go:2252-2265). A completion after resetting a scheduled workflow therefore remains targeted at the closed pre-reset run and never marks the scheduler action complete, potentially blocking later actions through overlap policy. The changed tests mask this by forcing envelope encoding on (tests/schedule_test.go:2795-2801), although its production default remains off (chasm/lib/callback/config.go:78-85); preserve the original start ID or gate this behavior on envelope support.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
in cloud callback.encodeInternalTokenWithEnvelope is set to true
dec7ca2 to
4eb6b8b
Compare
What changed?
Cherry-pick #12533 to cloud/v1.33.0-164
Had to modify tests, but no other changes besides that
Why?
Cherry-picking
How did you test it?
Potential risks
No