Repository navigation
fix(ha): claim workflows on spawn + persist running to stop the "untriggered" steal (#89) - #98
Merged
Merged
Conversation
…ggered steal (#89) On the multi-node HA stack a freshly triggered workflow intermittently stayed in "untriggered" forever: the trigger path never set claimed_by and never persisted the untriggered->running transition (only journaled it), so the row sat at claimed_by=NULL, state=untriggered for the whole run. The NOTIFY-driven claim sweep on other nodes then stole and re-triggered it, spawning a duplicate handler and stalling the run; recovery (running/sleeping only) never healed it. Fix: - ClaimRepository.ClaimWorkflow(nodeID, workflowID) — atomically claim a single workflow for the owning node (memory no-op returns true; postgres UPDATE ... RETURNING rows-affected). - WorkflowHandler.Init claims the workflow for this node on both the new-trigger and replay paths; on loss to another live node it bows out (tells the instance supervisor it's done here) so no duplicate runs. It then persists the running/sleeping state immediately so the DB row reflects reality. - Claim sweep lease-expired branch and recoverWorkflows now include 'untriggered' so a crash-stranded row is reclaimed/re-driven. - ADR-0018 amended with the ownership invariants. Non-HA/memory: no behavior change (claim is a no-op, claim actor is HA-gated). The e2e re-trigger quarantine stays as defense-in-depth until 3-node runs confirm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the root cause of #89 (the e2e quarantine was only a workaround). One PR to
main.Root cause
On the 3-node HA stack, a freshly triggered workflow intermittently stayed
untriggeredforever:claimed_byand never persisted theuntriggered → runningtransition (only journaled it), so the row sat at
claimed_by=NULL, state=untriggeredfor thewhole run;
NOTIFYon theuntriggeredINSERT makes every node's claim actor sweep immediately,and the sweep claims
claimed_by IS NULL AND state IN ('untriggered','running','sleeping')→ anothernode stole and re-triggered it → a duplicate
WorkflowHandler→ stall;recoverWorkflows(running/sleeping only) never healed it, andGET /v1/workflows/{id}reportedthe running workflow as
untriggered.Fix
ClaimRepository.ClaimWorkflow(nodeID, workflowID)— atomically claim a single workflow forthe owning node (memory no-op →
true; postgresUPDATE … RETURNINGrows-affected).WorkflowHandler.Initclaims the workflow for this node on both the new-trigger and replaypaths; on loss to another live node it bows out (signals the instance supervisor it's done
here) so no duplicate runs. It then persists the running/sleeping state immediately.
recoverWorkflowsnow includeuntriggeredso acrash-stranded row is reclaimed/re-driven.
Safety
Non-HA / memory: no behavior change (claim is a no-op; the claim actor + PgListener are HA-gated).
The e2e re-trigger quarantine stays as defense-in-depth until
make ha-upruns confirm zero stuckuntriggered.Verify
make lint0 ·make buildok ·make test701 pass · functional tests compile. Adds a memoryClaimWorkflowunit test and a Postgres contract subtest (claims, idempotent for owner, blocks otherlive nodes). The real 3-node assertion is the CI
e2etier (can't run a 3-node stack locally).🤖 Generated with Claude Code