Skip to content

Fix df.signal not propagated into child sub-orchestrations - #154

Merged
Pino de Candia (pinodeca) merged 2 commits into
mainfrom
copilot/fix-signal-propagation-issue
May 22, 2026
Merged

Fix df.signal not propagated into child sub-orchestrations#154
Pino de Candia (pinodeca) merged 2 commits into
mainfrom
copilot/fix-signal-propagation-issue

Conversation

Copilot AI commented May 19, 2026

Copy link
Copy Markdown
Contributor
  • Inspect current signal/race implementation and existing E2E tests
  • Check recent GitHub Actions workflow runs/log context for this issue
  • Run baseline local validation (format/build/tests) before code changes
  • Implement minimal fix so df.signal reaches relevant child sub-orchestrations (race/wait_for_signal case)
  • Add focused E2E coverage for wait_for_signal inside df.race
  • Run targeted validation for changed paths, then broader checks as needed
  • Run parallel validation (Code Review + CodeQL) and address any valid findings

Copy link
Copy Markdown
Contributor

Review followups from the deep-dive on this change. Filing them here so they aren't lost; all are intentionally deferred — either because they're properly solved by the upstream duroxide fix tracked in microsoft/duroxide#21, or because they're polish that doesn't gate this PR.

# Concern Where Status
1 Race: signal raised before the sub-orchestration is Running (or even scheduled) is delivered to the root only, lost when the child later subscribes. raise_external_event fan-out logic Deferred — proper fix needs unmatched-event forwarding in duroxide (microsoft/duroxide#21).
2 Pending descendants (scheduled but not yet picked up by the worker) are filtered out by the status == "running" check and miss the signal. list_running_descendants Deferred — same upstream fix removes the need to inspect child state at all.
3 Fan-out is a true broadcast: every running descendant receives every signal regardless of whether it has a matching schedule_wait. Harmless today but couples signal namespaces across unrelated branches. raise_external_event Deferred — subsumed by microsoft/duroxide#21 (option 1 / option 3 in that issue eliminate the broadcast).
4 N+1 round-trips per df.signal() call: one get_instance_tree + one get_instance_info per descendant. Fine for the common small-fanout case, would hurt for wide JOIN/RACE trees or hot signal paths. list_running_descendants Deferred — not worth optimizing the workaround; goes away with upstream fix.
5 Status check uses eq_ignore_ascii_case("running") against duroxide's free-form status string. Fragile if duroxide ever renames/recases the variant. list_running_descendants Deferred — same lifetime as the workaround.
6 continue_as_new on a parent does not GC prior-iteration sub-orchestration rows; long-running loops with a JOIN/RACE body accumulate child instances in duroxide.instances indefinitely, slowing get_instance_tree over time. Pre-existing duroxide behavior, surfaced by this PR's reliance on get_instance_tree. Deferred — track separately; not introduced by this PR.

Filed microsoft/duroxide#21 to request the upstream primitive (unmatched-event forwarding or an explicit raise_event_tree) that would replace this best-effort fan-out with a race-free, single-call delivery. Once that lands, this client-side workaround should be removed.

@pinodeca
Pino de Candia (pinodeca) merged commit f6b486a into main May 22, 2026
5 checks passed
@pinodeca
Pino de Candia (pinodeca) deleted the copilot/fix-signal-propagation-issue branch May 22, 2026 20:21
Pino de Candia (pinodeca) added a commit that referenced this pull request Jun 29, 2026
Tests 1, 3, 4, and 5 sent a single fire-once signal after a fixed
pg_sleep. duroxide drops any external event raised before the
wait_for_signal subscription is registered ("no pending subscription
slot", duroxide #154), so under CI/load the signal could land before
the subscription exists and the workflow would wait out its timeout,
causing df.await_instance to time out (status: running).

Replace the fixed-sleep + fire-once pattern with a retry loop that
re-raises the signal while the instance is still 'running' and stops
once it leaves that state, guaranteeing at least one signal lands after
the subscription is registered. Checking status before each send avoids
signaling an already-completed instance.

Verified with 10 consecutive repeats:
  ./scripts/test-e2e-local.sh 07_signals 10  -> 10 passed, 0 failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

df.signal not propagated into child sub-orchestrations (breaks wait_for_signal inside df.race)

2 participants