Skip to content

Replace orphaned child workflows with no worker progress after force failover - #11815

Merged
jiechenz merged 5 commits into
mainfrom
jiechenz/parent-child-ff-issue
Sep 2, 2026
Merged

Replace orphaned child workflows with no worker progress after force failover#11815
jiechenz merged 5 commits into
mainfrom
jiechenz/parent-child-ff-issue

Conversation

@jiechenz

@jiechenz jiechenz commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What changed?

  • Added request-only OrphanedChildReplacementInfo, carrying the parent’s current Version History as branch evidence.
  • When enabled, a retried child start may atomically terminate an orphaned conflicting run and create a replacement.
  • Replacement is allowed only when the conflicting child:
    • belongs to the same parent run;
    • was initiated on a losing parent branch, while the incoming initiation is on the current branch;
    • is the first run in its execution chain;
    • contains only WorkflowExecutionStarted;
    • has no pending in-memory Update.
  • Existing request-ID deduplication remains ahead of replacement handling.
  • Added outcome metrics, a post-commit success log, and the disabled-by-default history.enableOrphanedChildWorkflowReplacement setting.

Why?

After force failover, a child created from a losing parent branch may conflict with the same child start reissued by the winning branch. Normal workflow-ID conflict handling records WORKFLOW_ALREADY_EXISTS, leaving the parent unable to make progress and the original child orphaned.

Re-linking the existing child is unsafe because its parent coordinates are stored in immutable history. This change instead replaces it only when the new active cluster sees no progress beyond WorkflowExecutionStarted.

Ownership and progress are rechecked while holding the child lock, and termination plus replacement creation are committed atomically.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

Validated with:

  • go test -tags test_dep ./service/history/api ./service/history -count=1
  • Focused parent-child XDC integration test
  • make proto
  • make lint-code

Potential risks

  • The decision uses the new active cluster’s locally visible state. During a network partition, it cannot know whether the previous active advanced the child beyond the replicated state.
  • Replacement terminates an existing run. The feature is disabled by default and fails closed unless every ownership, branch, and progress check passes under the child lock.
  • The request carries the parent’s full current VersionHistoryItems list. An unusually long version history may increase RPC size and could exceed internal gRPC limits; oversized requests fail instead of recovering the child.
  • All history hosts processing child starts in an enabled cluster must understand the request field. An older host may ignore it and record WORKFLOW_ALREADY_EXISTS.

@jiechenz jiechenz added the team/cgs-foundation Require cgs foundation team review label Aug 27, 2026
@jiechenz
jiechenz marked this pull request as ready for review August 27, 2026 05:40
@jiechenz
jiechenz requested a review from a team August 27, 2026 05:40
@jiechenz
jiechenz requested review from a team as code owners August 27, 2026 05:40
case enumsspb.WORKFLOW_EXECUTION_STATE_CREATED:
return mutableState.GetNextEventID() == common.FirstEventID+1
case enumsspb.WORKFLOW_EXECUTION_STATE_RUNNING:
if mutableState.GetNextEventID() != common.FirstEventID+2 || mutableState.HasCompletedAnyWorkflowTask() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does check if there is any workflow task schedule good enough? e.g. use HadOrHasWorkflowTask?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's a bit nasty because the goal is to capture WORKFLOW_TASK_SCHEDULED event but not started. Not semantically same to HadOrHasWorkflowTask() check. I have narrowed this check to only WORKFLOW_EXECUTION_STARTED event, where it matched the incident.


func isOrphanedChildWithoutProgress(mutableState historyi.MutableState) bool {
executionState := mutableState.GetExecutionState()
if executionState.GetFirstExecutionRunId() != mutableState.GetWorkflowKey().RunID {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if the child continue as new, will it has a different run id to the first execution run id?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes CAN will have a new runID. It on purpose to skip handle the CAN / Cron etc runs. Only when the conflicted run is a first-time run and hasn't progressed, we can safely terminate and replace.

Allow a parent to mark an open child from a losing initiation as
zombie and create its replacement atomically when the reuse policy
permits.

Validate ownership and initiation metadata under the child lock while
preserving normal deduplication for accepted children.
@jiechenz
jiechenz requested a review from yux0 August 27, 2026 22:39
@jiechenz
jiechenz force-pushed the jiechenz/parent-child-ff-issue branch from 9d1d347 to 44cffe5 Compare August 27, 2026 22:54
if executionState.GetFirstExecutionRunId() != mutableState.GetWorkflowKey().RunID ||
executionState.GetState() != enumsspb.WORKFLOW_EXECUTION_STATE_CREATED ||
mutableState.GetNextEventID() != common.FirstEventID+1 ||
workflowLease.GetContext().UpdateRegistry(ctx).Len() != 0 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we just check the pending updates and received signal count?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think firstRunID check is needed for not terminating successors of CAN / cron.
The state = CREATED check is also required to make sure no WFT scheduled.

For external writes, such as Pause, Cancel and Signal, they all append history events. I believe checking on FirstEventID + 1 could cover all of them.

@jiechenz
jiechenz requested a review from yux0 September 2, 2026 17:45
@jiechenz jiechenz added the reliability-2026 Reliability related changes label Sep 2, 2026
@jiechenz
jiechenz merged commit a07a006 into main Sep 2, 2026
58 checks passed
@jiechenz
jiechenz deleted the jiechenz/parent-child-ff-issue branch September 2, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reliability-2026 Reliability related changes team/cgs-foundation Require cgs foundation team review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants