Skip to content

[Fix] Controller shutdown hangs when worker launches stall - #3445

Merged
daniel-lxs merged 1 commit into
developfrom
fix/bounded-controller-stop-2ausc3ucazj2w
Oct 9, 2026
Merged

daniel-lxs merged 1 commit into
developfrom
fix/bounded-controller-stop-2ausc3ucazj2w

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

​Opened on behalf of @daniel-lxs. Follow up by mentioning @roomote-roomote, in the web UI, or in Telegram.

What changed

Controller shutdown now spends one existing wait budget across the current dequeue iteration and in-flight worker launches. A stalled provider launch no longer prevents watcher cleanup and teardown from starting. Deadline warnings identify unfinished run IDs, normal completion clears its timers, and the progress interval cannot keep the process alive by itself.

Late queue, database-claim and provider-resolution results do not admit new provider work after shutdown begins. Their durable Pending/Dequeued rows remain eligible for the existing recovery path. Skipped dispatch is not logged as a successful worker launch.

Why this change was made

The current iteration wait was bounded, but the following Promise.allSettled over provider launches was not. Since SIGTERM/SIGINT handling awaits stop(), a never-settling provider promise could hang graceful shutdown indefinitely. Bounding the second wait also requires guarding late results so cleanup cannot be followed by a fresh dispatch.

Impact

The existing 60-second budget limits the waits before controller teardown; no new timeout setting is added. Task runs are not failed, canceled or completed because shutdown times out. Provider operations already in progress are not canceled or remotely fenced, and may settle while the process is alive. Teardown itself retains its existing behavior and is not given a new deadline. The controller entrypoint exits after stop/flush; persisted recovery leases remain authoritative.

How it was tested

  • Current develop 35201fbf: actual controller start/dequeue with real Postgres and Redis, controlled queue/provider/auth/monitoring. With a 50ms test budget, stop remained pending at 125ms, teardown had not run, and the run/event were durably Dequeued/nonterminal; releasing the provider allowed stop to finish.
  • Exact pushed head a6944e453e1ff32e58d678c617e592345af268ec: repeated independent-process proof on Node 24.13.1. A hung-provider child naturally exited without force after stop (66ms including test client teardown with a 50ms wait budget). Parent reads confirmed Dequeued state, an actual dequeue event and no terminal/cancellation writes. Two separate recovery processes concurrently reclaimed exactly once and renewed the persisted lease.
  • Independent late-result cases: a Redis pop after stop leaves Pending with zero dequeue events/provider calls; a database claim completing after stop leaves Dequeued with one actual event and zero provider calls. Both children exit naturally. Real disposable database creation/schema bootstrap/drop and isolated Redis cleanup completed.
  • 46 focused/adjacent controller tests passed, including one shared budget, normal settlement/timer cleanup, late Redis/claim guards, existing orphan recovery and provider lifecycle handling. Controller bundle build, fast types, broad fast lint/types/Knip and normal pre-push hooks passed. Precommit Judgement passed four judgments.
  • Hosted full CI passed, including Test, deployment artifacts, app image, backup/restore, upgrade compatibility and CodeQL. Worker-image matrix is an expected path skip. Exact-head automated review is clean with zero findings/threads. Hosted Judgement is green by policy but explicitly incomplete with zero judgments in the readiness run; precommit's four judgments are separate evidence, not complete hosted semantic coverage.
  • Queue/provider/auth/monitoring collaborators are controlled; actual controller code, SQL transitions, persistence, Redis, independent-process cleanup and recovery claims remain in the path. No live compute-provider parity, remote-operation fencing or bound on teardown itself is claimed. Browser proof is not applicable; no screenshots/video are embedded. This PR is ready for human review, not approved or merge-certified.

Checklist

  • The PR title follows the repo convention: [Fix], [Feat], [Improve], [Refactor], [Docs], or [Chore] followed by a user-facing description
  • This PR is small and scoped to one change
  • pnpm lint and pnpm check-types pass locally — normal fast repository gates passed; full formatting-inclusive commands were not run
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset

Related PRs

These are independent reliability fixes from the same shift; none depends on another.

@roomote-community

roomote-community Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

No code issues found. See task

Reviewed the shared shutdown deadline, timer cleanup, late-result dispatch guards, spawn callers, teardown and persisted orphan recovery. Current-head test, lint, type-check and Knip checks passed; Docker build and JavaScript/TypeScript CodeQL were still running when checked.

Reviewed a6944e4

@daniel-lxs daniel-lxs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verified stalled-launch shutdown through real controller processes, queue and database state, plus concurrent durable orphan recovery with a controlled provider. Focused lifecycle tests, controller checks/build and current CI pass at this head.

@daniel-lxs
daniel-lxs merged commit 425f343 into develop Oct 9, 2026
29 checks passed
@daniel-lxs
daniel-lxs deleted the fix/bounded-controller-stop-2ausc3ucazj2w branch October 9, 2026 23:19
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.

1 participant