-
Notifications
You must be signed in to change notification settings - Fork 8k
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
fix(core): Account for waiting jobs during shutdown #11338
Conversation
@@ -126,13 +126,23 @@ export class ScalingService { | |||
|
|||
@OnShutdown(HIGHEST_SHUTDOWN_PRIORITY) | |||
async stop() { | |||
await this.queue.pause(true, true); // no more jobs will be picked up | |||
const { instanceType } = this.instanceSettings; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original issue was main waiting forever on waiting (not running) executions when there were no workers available during the 30s grace period during shutdown. The deeper issue was that main should not be waiting at all for already enqueued jobs during the grace period, i.e. we need to divide responsibilities during shutdown based on instance type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't really reason what all the implications of this change might be, but at least it should make the shutdown work better 👍
n8n Run #7498
Run Properties:
|
Project |
n8n
|
Branch Review |
pay-2129
|
Run status |
Passed #7498
|
Run duration | 04m 28s |
Commit |
71c670d518: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 ivov 🗃️ e2e/*
|
Committer | Iván Ovejero |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
1
|
Pending |
0
|
Skipped |
0
|
Passing |
458
|
View all changes introduced in this branch ↗︎ |
✅ All Cypress E2E specs passed |
Got released with |
https://linear.app/n8n/issue/PAY-2129