-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
perf(core): Batch workflow activation to speed up startup #13191
Conversation
@@ -533,7 +543,6 @@ export class ActiveWorkflowManager { | |||
} | |||
|
|||
if (shouldDisplayActivationMessage) { | |||
this.logger.info(` - ${dbWorkflow.display()}`); |
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.
Move next to its pair to prevent logs out of order:
================================
Start Active Workflows:
================================
- "My workflow 70" (ID: rriRxvVT6Y1YcUnq))
- "My workflow 71" (ID: UNlbMpRODEH9OkW2))
=> Started
=> Started
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
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.
Couple comments
const activationPromises = batch.map(async (dbWorkflow) => { | ||
try { | ||
const wasActivated = await this.add(dbWorkflow.id, activationMode, dbWorkflow, { | ||
shouldPublish: false, | ||
}); | ||
this.logger.info(' => Started'); | ||
} | ||
} catch (error) { | ||
this.errorReporter.error(error); | ||
this.logger.info( | ||
' => ERROR: Workflow could not be activated on first try, keep on trying if not an auth issue', | ||
); | ||
if (wasActivated) { | ||
this.logger.info(` - ${dbWorkflow.display()})`); | ||
this.logger.info(' => Started'); | ||
this.logger.debug(`Successfully started workflow ${dbWorkflow.display()}`, { | ||
workflowName: dbWorkflow.name, | ||
workflowId: dbWorkflow.id, | ||
}); | ||
} | ||
} catch (error) { | ||
this.errorReporter.error(error); | ||
this.logger.info( | ||
` => ERROR: Workflow ${dbWorkflow.display()} could not be activated on first try, keep on trying if not an auth issue`, | ||
); | ||
|
||
this.logger.info(` ${error.message}`); | ||
this.logger.error( | ||
`Issue on initial workflow activation try of ${dbWorkflow.display()} (startup)`, | ||
{ | ||
workflowName: dbWorkflow.name, | ||
workflowId: dbWorkflow.id, | ||
}, | ||
); | ||
this.logger.info(` ${error.message}`); | ||
this.logger.error( | ||
`Issue on initial workflow activation try of ${dbWorkflow.display()} (startup)`, | ||
{ | ||
workflowName: dbWorkflow.name, | ||
workflowId: dbWorkflow.id, | ||
}, | ||
); | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument | ||
this.executeErrorWorkflow(error, dbWorkflow, 'internal'); | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument | ||
this.executeErrorWorkflow(error, dbWorkflow, 'internal'); | ||
|
||
// do not keep trying to activate on authorization error | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-call | ||
if (error.message.includes('Authorization')) continue; | ||
// do not keep trying to activate on authorization error | ||
// eslint-disable-next-line @typescript-eslint/no-unsafe-call | ||
if (error.message.includes('Authorization')) return; | ||
|
||
this.addQueuedWorkflowActivation('init', dbWorkflow); | ||
} | ||
this.addQueuedWorkflowActivation('init', dbWorkflow); | ||
} | ||
}); |
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.
Could we extract this into a separate method to make this more readable? Either the creation of a batch or activation of a single WF
for (let i = 0; i < dbWorkflows.length; i += activationBatchSize) { | ||
const batch = dbWorkflows.slice(i, i + activationBatchSize); |
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.
Using e.g. lodash chunk
would be clearer than implementing the chunking ourselves
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.
Nice work 👏 🚀
|
n8n
|
Project |
n8n
|
Branch Review |
batch-activation
|
Run status |
|
Run duration | 04m 36s |
Commit |
|
Committer | Iván Ovejero |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
1
|
|
5
|
|
0
|
|
436
|
View all changes introduced in this branch ↗︎ |
✅ All Cypress E2E specs passed |
Got released with |
Summary
During startup we activate workflows one by one, which can be slow for instances with many workflows to activate. This PR introduces support for batching workflow activations via
N8N_WORKFLOW_ACTIVATION_BATCH_SIZE
so users can choose how much of this work to parallelize based on their use cases.Related Linear tickets, Github issues, and Community forum posts
n/a
Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)