mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-19 16:55:02 +02:00
perf(editor): gate node search behind a per-user rate limit and process-wide concurrency cap
Ports the multi-tenant containment from #30294: - GET /workflows/search-nodes is rate limited per user (120/min); the client debounces its keystrokes, which no human can sustain past this ceiling. - The service runs at most one scan per process (pLimit(1)) with a short queue of 4, shedding excess with 429. A per-user rate limit cannot bound load on a shared database: #30294 measured 66x (SQLite) / 56x (Postgres) workflow-list degradation from 20 users each inside their own limit; capped at one concurrent scan the same scenario measured ~1.3x. In multi-main setups the ceiling is per instance, which is the intent: it bounds what a single process can demand of the shared DB. The command-bar composable already treats a failed request as empty results, so a shed search degrades gracefully client-side. Verified: service unit 19/19, integration 23/23 (new shed test) on both SQLite and Postgres 16; controller unit 11/11; typecheck + lint clean.
This commit is contained in:
parent
d24fa54996
commit
35fe5daef1
|
|
@ -10,7 +10,9 @@ import { WorkflowRepository } from '@n8n/db';
|
|||
import { Service } from '@n8n/di';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import { findNodeSearchMatch, STICKY_NODE_TYPE } from 'n8n-workflow';
|
||||
import pLimit from 'p-limit';
|
||||
|
||||
import { TooManyRequestsError } from '@/errors/response-errors/too-many-requests.error';
|
||||
import { RoleService } from '@/services/role.service';
|
||||
|
||||
/**
|
||||
|
|
@ -30,6 +32,25 @@ const BATCH_SIZE = 100;
|
|||
*/
|
||||
const MAX_SCANNED_WORKFLOWS = 2000;
|
||||
|
||||
/**
|
||||
* A query that matches nothing cannot short-circuit on the `updatedAt` index — it
|
||||
* has to read every workflow's `nodes` blob to prove absence, and the keyset
|
||||
* scan's worst case walks MAX_SCANNED_WORKFLOWS of hydrated false positives.
|
||||
*
|
||||
* The per-user rate limit on the endpoint cannot bound that, because the
|
||||
* database is shared. Measured in #30294 on a 20k-workflow corpus: with 20
|
||||
* users each staying inside their own limit, workflow-list queries degraded
|
||||
* 66x on SQLite and 56x on Postgres; capped at one concurrent scan the same
|
||||
* scenario measures ~1.3x.
|
||||
*
|
||||
* So run one scan at a time per process and shed load past a short queue
|
||||
* rather than letting search monopolise the connection pool. In multi-main
|
||||
* setups the ceiling is per instance, which is the intent: it bounds what a
|
||||
* single process can demand of a shared database.
|
||||
*/
|
||||
const MAX_CONCURRENT_SEARCHES = 1;
|
||||
const MAX_QUEUED_SEARCHES = 4;
|
||||
|
||||
/** Relevance order for matched fields — a node-name hit beats a buried parameter hit. */
|
||||
const MATCHED_FIELD_RANK: Record<NodeSearchMatchedField, number> = {
|
||||
name: 0,
|
||||
|
|
@ -48,10 +69,15 @@ export class WorkflowNodeSearchService {
|
|||
private readonly roleService: RoleService,
|
||||
) {}
|
||||
|
||||
private readonly limiter = pLimit(MAX_CONCURRENT_SEARCHES);
|
||||
|
||||
/**
|
||||
* Full-text search over the nodes of every non-archived workflow the user can
|
||||
* read. Matches node names, types, notes and parameter values.
|
||||
* Optionally restrict to workflows owned by `projectId`.
|
||||
* read. Matches node names, types, notes, parameter values and credential
|
||||
* names. Optionally restrict to workflows owned by `projectId`.
|
||||
*
|
||||
* Throws `TooManyRequestsError` when the process-wide concurrency gate and
|
||||
* its queue are full — see MAX_CONCURRENT_SEARCHES.
|
||||
*/
|
||||
async search(
|
||||
user: User,
|
||||
|
|
@ -61,6 +87,18 @@ export class WorkflowNodeSearchService {
|
|||
const query = rawQuery.trim();
|
||||
if (!query) return { results: [], hasMore: false };
|
||||
|
||||
if (this.limiter.pendingCount >= MAX_QUEUED_SEARCHES) {
|
||||
throw new TooManyRequestsError('Workflow node search is busy, please retry.');
|
||||
}
|
||||
|
||||
return await this.limiter(async () => await this.runSearch(user, query, options));
|
||||
}
|
||||
|
||||
private async runSearch(
|
||||
user: User,
|
||||
query: string,
|
||||
options: { projectId?: string },
|
||||
): Promise<SearchWorkflowNodesResponse> {
|
||||
const scopes = ['workflow:read' as const];
|
||||
const [projectRoles, workflowRoles] = await Promise.all([
|
||||
this.roleService.rolesWithScope('project', scopes),
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ import {
|
|||
} from '@n8n/db';
|
||||
import {
|
||||
Body,
|
||||
createUserKeyedRateLimiter,
|
||||
Delete,
|
||||
Get,
|
||||
Licensed,
|
||||
|
|
@ -164,8 +165,14 @@ export class WorkflowsController {
|
|||
* every project the caller has access to, so there is no single scope to check.
|
||||
* Access is enforced inside the query itself, via the shared-workflow subquery
|
||||
* restricted to `workflow:read`.
|
||||
*
|
||||
* Each call runs an unindexable substring scan, so it is rate limited per
|
||||
* user: the client debounces its keystrokes, which no human can sustain past
|
||||
* this ceiling. Cross-user load is bounded by the service's concurrency gate.
|
||||
*/
|
||||
@Get('/search-nodes')
|
||||
@Get('/search-nodes', {
|
||||
keyedRateLimit: createUserKeyedRateLimiter({ limit: 120, windowMs: 60_000 }),
|
||||
})
|
||||
async searchNodes(
|
||||
req: AuthenticatedRequest,
|
||||
_res: express.Response,
|
||||
|
|
|
|||
|
|
@ -234,6 +234,31 @@ describe('WorkflowNodeSearchService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('load shedding', () => {
|
||||
// The per-user rate limit cannot bound load on a shared database: 20 users
|
||||
// each staying within their own limit measured 66x list-query degradation
|
||||
// before the process-wide cap existed (#30294).
|
||||
it('sheds load once the concurrency cap and its queue are full', async () => {
|
||||
await createWorkflow({ nodes: [makeNode({ name: 'Send Slack Alert' })] }, member);
|
||||
|
||||
// Fire more than (max concurrent + max queued) at once. The guard is
|
||||
// evaluated synchronously before the limiter awaits, so this is deterministic.
|
||||
const outcomes = await Promise.allSettled(
|
||||
Array.from({ length: 10 }, async () => await service.search(member, 'slack')),
|
||||
);
|
||||
|
||||
const rejected = outcomes.filter((outcome) => outcome.status === 'rejected');
|
||||
const fulfilled = outcomes.filter((outcome) => outcome.status === 'fulfilled');
|
||||
expect(rejected.length).toBeGreaterThan(0);
|
||||
expect(fulfilled.length).toBeGreaterThan(0);
|
||||
expect(rejected[0].reason).toMatchObject({ httpStatusCode: 429 });
|
||||
|
||||
// The cap must not wedge the service: later calls succeed normally.
|
||||
const { results } = await service.search(member, 'slack');
|
||||
expect(results).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('LIKE metacharacters', () => {
|
||||
it('treats % as a literal', async () => {
|
||||
await createWorkflow({ nodes: [makeNode({ name: 'Discount 50% off' })] }, member);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user