temporalio / temporalio/temporal

IsTerminatedByResetter should not be used for checking if child workflow should report to parent

Open
#6,954 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

potential-bug
Dominant language
Go
Stars
23.2k
Forks
1.9k
Avg merge
2d 8h
Merged PRs (30d)
228

Description

Expected Behavior

  • A reset operation involves three runs: base run, current run and reset (new) run. If current run is running when processing a reset, it will be terminated. If current run is also a child workflow, then there's a question if it should report to its parent after the termination.
  • If current run is base run, then we should not report to parent, as we will have a new run for the child.
  • If current run is not base run, then we should report to parent as the new run is not a continuation of the current run.

Actual Behavior

  • In the replyToParentWorkflow logic, we are checking if the current run is terminated by reset or not, which is not checking if current run is the base run.

We should use the new ResetRunId field in WorkflowExecutionInfo, which is only set when the workflow is the base run of a reset. However, note that with today's event-based replication stack, this ResetRunID field won't be replicated upon reset. So if we switch to that field now, we will temporary make things worse (if failover happens before the closeExecution task is processed) until state-based replication goes live.

To fix the issue before state-based replication, we can:

  1. Change existing replication logic and instead of generating a historyReplicationTask when workflow is closed, generate a SyncWorkflowState task (yes we have this sync state task in event-based world as well, but only works for closed workflow)
  2. Add some new fields to the Terminated event to indicate if the workflow is base run.

Steps to Reproduce the Problem

Specifications

  • Version:
  • Platform:

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in service/history/transfer_queue_active_task_executor.go at the replyToParentWorkflow logic, then trace WorkflowExecutionInfo, reset handling, and the event-based replication path. Compare the base-run and non-base-run cases, including failover before closeExecution is processed. Done means parent reporting follows the base-run rule without losing the required reset state during replication.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
distributed-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.