perf(core): Introduce concurrency control for main mode #9453
Conversation
n8n-assistant
bot
added
core
| release({ mode }: { mode: ExecutionMode }) { | ||
| if (!this.isEnabled || this.isUncapped(mode)) return; | ||
|
|
||
| this.productionQueue.dequeue(); |
There was a problem hiding this comment.
would it be better if dequeue explicitly removed a specific executionId from the queue, instead of simply saying that "one slot just opened up" 🤔 ?
There was a problem hiding this comment.
Sorry I'm not seeing what you're seeing here - what is the benefit of this? remove() removes by execution ID but that's only needed when deleting an execution.
packages/cli/src/License.ts
Outdated
| @@ -357,6 +357,12 @@ export class License { | |||
| return this.getFeatureValue(LICENSE_QUOTAS.TEAM_PROJECT_LIMIT) ?? 0; | |||
| } | |||
|
|
|||
| getConcurrencyProductionCap() { | |||
| return ( | |||
| this.getFeatureValue(LICENSE_QUOTAS.CONCURRENCY_PRODUCTION_CAP) ?? UNLIMITED_LICENSE_QUOTA | |||
There was a problem hiding this comment.
have we discussed that we want to use the license to determine concurrency limits? if yes, do we still want to use the executions.concurrency.productionCap config variable?
It feels like one or the other should suffice.
There was a problem hiding this comment.
So from reading this, I can only see mentions to creating environment variables, nothing license related.
For cloud where we want to impose some limits (to safeguard user instances) this can be done via environment variables, there's no need for any sort of licensing here, right?
There was a problem hiding this comment.
We could impose those limits using the ephemeral licenses but that's not necessary, we can simply use environment variables for this, unless we really want to centralize it all with licenses, but it seems to me we're mixing requirements
|
@ivov It would be great if we could configure a worker to only run workflows that have certain tags. This way we can configure a service for the "critical" tag, another for the "mailing" tag, etc... |
| @@ -357,6 +357,12 @@ export class License { | |||
| return this.getFeatureValue(LICENSE_QUOTAS.TEAM_PROJECT_LIMIT) ?? 0; | |||
| } | |||
|
|
|||
| getConcurrencyProductionLimit() { | |||
| return ( | |||
| this.getFeatureValue(LICENSE_QUOTAS.CONCURRENCY_PRODUCTION_LIMIT) ?? UNLIMITED_LICENSE_QUOTA | |||
There was a problem hiding this comment.
I don't think we should be limiting concurrency via license.
That's a product discussion but I'd say we should toggle on or off the feature via license, but the number of concurrent executions may be handled by an environment variable right? Just making sure we have the right specs here.
There was a problem hiding this comment.
It's part of the reqs, please see here: #9453 (comment)
packages/cli/src/PublicApi/v1/handlers/executions/executions.handler.ts
Outdated
Show resolved
Hide resolved
packages/cli/src/databases/repositories/execution.repository.ts
Outdated
Show resolved
Hide resolved
This PR introduces concurrency control for main mode. This limits how many production executions are allowed to run at the same time in main mode, preventing event loop overload and so improving reliability.
Concurrency control applies only in main mode and only to production executions (i.e. in
webhookandtriggermodes), not to executions inmanual,retry,error,integrated,cliorinternalmodes.Summary of changes
Queuing
ConcurrencyControlService.ConcurrencyQueue.ActiveExecutions.startedAttimestamp.runningovernewEnvs, logs, tests
N8N_CONCURRENCY_PRODUCTION_CAP.quota:maxConcurrentProductionExecutions, allowing for override withN8N_CLOUD_OVERRIDE_CONCURRENCY_PRODUCTION_CAP.ConcurrencyControlServiceandConcurrencyQueue.Discussion: https://www.notion.so/n8n/Concurrency-limits-for-main-mode-8b642df5e98a479cadd1cfc4950d27a1
Follow-up to: #8458