Skip to content

perf(core): Don't load task-runner on main instances when manual executions are offloaded to workers #15986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

netroy
Copy link
Contributor

@netroy netroy commented Jun 3, 2025

Summary

When manual executions offloading is enabled, there are no executions on main instances, and therefore there is no need to be running a task-runner subprocess.

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@netroy netroy requested a review from ivov June 3, 2025 16:28
@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Jun 3, 2025
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

cubic reviewed 1 file and found no issues. Review PR in cubic.dev.

Copy link

codecov bot commented Jun 3, 2025

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/cli/src/commands/start.ts 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment on lines +200 to +203
if (process.env.OFFLOAD_MANUAL_EXECUTIONS_TO_WORKERS === 'true') {
this.needsTaskRunner = false;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Should add a test for this? There's a very similar one at packages/cli/src/commands/__tests__/execute.test.ts

Copy link
Contributor

Choose a reason for hiding this comment

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

Gave it a try but as usual commands are painful to test, so this little change is not worth the time. Let's disentangle this command some other time.

Copy link
Contributor

github-actions bot commented Jun 6, 2025

✅ All Cypress E2E specs passed

@ivov ivov changed the title fix(core): Don't load task-runner on main instances when manual executions are offloaded to workers perf(core): Don't load task-runner on main instances when manual executions are offloaded to workers Jun 6, 2025
@ivov ivov merged commit 23ce60d into master Jun 6, 2025
69 checks passed
@ivov ivov deleted the no-task-runner-in-main-when-manual-exeutions-are-offloaded branch June 6, 2025 08:28
@janober
Copy link
Member

janober commented Jun 11, 2025

Got released with [email protected]

TianYi0217 pushed a commit to TianYi0217/n8n that referenced this pull request Jun 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants