Skip to content

Commit

Permalink
fix(core): Consider timeout in shutdown an error (#8050)
Browse files Browse the repository at this point in the history
If the process doesn't shutdown within a time limit, exit with error
code.

1. conceptually something timing out is an error.
2. on successful exit we close down the DB connection gracefully. On an
exit timeout we rather not do that, since it will wait for any active
connections to close and would possible block the exit.
  • Loading branch information
tomi authored Dec 18, 2023
1 parent aad57e2 commit 4cae976
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 24 deletions.
17 changes: 17 additions & 0 deletions packages/cli/src/commands/BaseCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ export abstract class BaseCommand extends Command {

protected isShuttingDown = false;

/**
* How long to wait for graceful shutdown before force killing the process.
* Subclasses can override this value.
*/
protected gracefulShutdownTimeoutInS: number = 30;

async init(): Promise<void> {
await initErrorHandling();
initExpressionEvaluator();
Expand Down Expand Up @@ -309,9 +315,20 @@ export abstract class BaseCommand extends Command {
return;
}

const forceShutdownTimer = setTimeout(async () => {
// In case that something goes wrong with shutdown we
// kill after timeout no matter what
console.log(`process exited after ${this.gracefulShutdownTimeoutInS}s`);
const errorMsg = `Shutdown timed out after ${this.gracefulShutdownTimeoutInS} seconds`;
await this.exitWithCrash(errorMsg, new Error(errorMsg));
}, this.gracefulShutdownTimeoutInS * 1000);

this.logger.info(`Received ${signal}. Shutting down...`);
this.isShuttingDown = true;

await this.stopProcess();

clearTimeout(forceShutdownTimer);
};
}
}
7 changes: 0 additions & 7 deletions packages/cli/src/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,6 @@ export class Start extends BaseCommand {

await this.externalHooks?.run('n8n.stop', []);

setTimeout(async () => {
// In case that something goes wrong with shutdown we
// kill after max. 30 seconds no matter what
console.log('process exited after 30s');
await this.exitSuccessFully();
}, 30000);

// Shut down License manager to unclaim any floating entitlements
// Note: While this saves a new license cert to DB, the previous entitlements are still kept in memory so that the shutdown process can complete
await Container.get(License).shutdown();
Expand Down
6 changes: 0 additions & 6 deletions packages/cli/src/commands/webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,6 @@ export class Webhook extends BaseCommand {
try {
await this.externalHooks?.run('n8n.stop', []);

setTimeout(async () => {
// In case that something goes wrong with shutdown we
// kill after max. 30 seconds no matter what
await this.exitSuccessFully();
}, 30000);

// Wait for active workflow executions to finish
const activeExecutionsInstance = Container.get(ActiveExecutions);
let executingWorkflows = activeExecutionsInstance.getActiveExecutions();
Expand Down
13 changes: 3 additions & 10 deletions packages/cli/src/commands/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,21 +81,13 @@ export class Worker extends BaseCommand {
try {
await this.externalHooks?.run('n8n.stop', []);

const maxStopTime = config.getEnv('queue.bull.gracefulShutdownTimeout') * 1000;

const stopTime = new Date().getTime() + maxStopTime;

setTimeout(async () => {
// In case that something goes wrong with shutdown we
// kill after max. 30 seconds no matter what
await this.exitSuccessFully();
}, maxStopTime);
const hardStopTime = Date.now() + this.gracefulShutdownTimeoutInS;

// Wait for active workflow executions to finish
let count = 0;
while (Object.keys(Worker.runningJobs).length !== 0) {
if (count++ % 4 === 0) {
const waitLeft = Math.ceil((stopTime - new Date().getTime()) / 1000);
const waitLeft = Math.ceil((hardStopTime - Date.now()) / 1000);
this.logger.info(
`Waiting for ${
Object.keys(Worker.runningJobs).length
Expand Down Expand Up @@ -272,6 +264,7 @@ export class Worker extends BaseCommand {
}

async init() {
this.gracefulShutdownTimeoutInS = config.getEnv('queue.bull.gracefulShutdownTimeout');
await this.initCrashJournal();

this.logger.debug('Starting n8n worker...');
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ export const schema = {
env: 'QUEUE_RECOVERY_INTERVAL',
},
gracefulShutdownTimeout: {
doc: 'How long should n8n wait for running executions before exiting worker process',
doc: 'How long should n8n wait for running executions before exiting worker process (seconds)',
format: Number,
default: 30,
env: 'QUEUE_WORKER_TIMEOUT',
Expand Down

0 comments on commit 4cae976

Please sign in to comment.