fix(core): Wait for detached sub-workflows before the worker stops its task runner (backport to release-candidate/2.37.x) - #37888
Draft
n8n-assistant[bot] wants to merge 1 commit into
Conversation
Contributor
Author
|
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-37737-to-release-candidate/2.37.x
git worktree add --checkout .worktree/backport-37737-to-release-candidate/2.37.x backport-37737-to-release-candidate/2.37.x
cd .worktree/backport-37737-to-release-candidate/2.37.x
git reset --hard HEAD^
git cherry-pick -x afc4a54798bbed080dad6b68231655a34d4ed14d
git push --force-with-lease |
5 tasks
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.
Description
Backport of #37737 to
release-candidate/2.37.x.Checklist for the author (@sovietspaceship) to go through.
After this PR has been merged, it will be picked up in the next patch release for release track.
Original description
Summary
A worker that receives SIGTERM loses a sub-workflow started with "wait for sub-workflow completion" turned off. The child execution fails at its next Code node with
Task request timed out. The parent is recorded assuccess, so only the child carries the error.The worker runs its
@OnShutdownhooks in priority order, and only callsstopProcess()after all of them finish.ScalingService.stopWorker()runs first, at the highest priority, but waits only ongetRunningJobsCount(). A detached child has no queue job of its own, so that wait returns at once.TaskRunnerModule.stop()then shuts the task runner down at the default priority.ActiveExecutions.shutdown()does wait for the child, but it runs fromstopProcess(), after every hook, so it waits for an execution whose task runner is already gone. The child asks the draining task broker for a task, the broker refuses it immediately, and the requester reports that refusal as a timeout.This PR waits for the running in-process executions in the same hook that waits for queued jobs, so the task runner stays available until they finish.
ActiveExecutionsgainsgetRunningExecutionIds()andcancelRunningExecutions(), andExecutionRepositorygainscancelManyRunning().ScalingService.stopWorker()waits on both sources. The queued-job wait keeps its previous behaviour and stays unbounded. Only the new wait carries a budget, 80% of the graceful shutdown timeout, the same heuristic the task-broker drain uses.runningwith nostoppedAt. The engine's later write is conditional on the execution not being cancelled, so it becomes a no-op. The write matches onstatusas well as ID, so an execution that reached a terminal status after the ID snapshot is left alone. Its deadline is half of what the shutdown window still has, capped at three seconds, so it cannot outlive the force-exit timer.QUEUE_WORKER_TIMEOUTnow sets both the field that arms the force-exit timer and the field the drains budget against. Before, it moved one and not the other, so a drain could plan a window longer than the timer that ends the process. It also rejects a malformed value rather than reading a leading number out of it, matching how the config layer parsesN8N_GRACEFUL_SHUTDOWN_TIMEOUT.Net effect: a detached sub-workflow that finishes inside the shutdown window now completes, and the worker exits cleanly.
Notes for reviewers:
resolveSubworkflowDeadlinegives it a deadline independent of its parent. A child that outlives the budget still fails.How to test
Unit tests:
pnpm --filter=n8n test src/scaling/__tests__/scaling.service.test.ts src/__tests__/active-executions.test.ts src/commands/__tests__/worker.test.ts.Integration test for the new repository method:
pnpm --filter=n8n test:integration test/integration/database/repositories/execution.repository.test.ts.Manual, in queue mode with a worker:
Expected: the worker logs
Waiting for 1 in-process executions to finish..., the task runner stops only after the child finishes, and the child completes withsuccess.Verified on a real worker. Before: the child failed at the Code node with
Task request timed outandelapsedSeconds0, andFailed to get Redis contextfollowed. After: the child finishedsuccessin 21.0s, and the runner stopped 141ms after it. An ordinary queued execution drains as it did before.The expiry path was verified separately, with
N8N_GRACEFUL_SHUTDOWN_TIMEOUT=10against a child needing about 20s. The drain waits 8s, logs the timeout and the cancelled execution IDs, and the child is recordedcanceledwith astoppedAt. The worker exits 0 without reaching the force-exit path.Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/CAT-4349
Review / Merge checklist
Backport to Beta,Backport to Stable, orBackport to v1(if the PR is an urgent fix that needs to be backported)