mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 13:39:44 +02:00
perf(editor): Keep sub-workflow progress off the execution hot path
Two costs that scaled with the number of sub-executions rather than with anything visible, both of which a parent loop over a few hundred items would have multiplied. The progress arc no longer trickles between steps. The trickle needed a 1s interval, and because the composable was called unconditionally, every default node on the canvas ran one whether a sub-workflow was executing or not. The arc's own reason for existing was weak once the point was granted that the rotating running border already shows the node is alive, so this drops the behaviour rather than gating the timer. The reachable-node count is now computed on first emit instead of at hook registration, and node events return early when no parent pushRef resolves. Executions with no editor session — the overwhelming majority — no longer walk the child workflow's graph at all. Nodes are still tallied before that bail-out, so an editor attaching mid-run sees a truthful count. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
eec68f3f45
commit
aa7edc2b03
|
|
@ -2314,6 +2314,33 @@ describe('Execution Lifecycle Hooks', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('counts nodes that ran before an editor session attached', async () => {
|
||||
// Progress work is skipped while nothing is watching, but the tally must
|
||||
// not be: an editor attaching mid-run should see a truthful count.
|
||||
activeExecutions.has.mockReturnValue(false);
|
||||
const hooks = buildHooks(
|
||||
buildChildWorkflow({
|
||||
nodes: ['Node A', 'Node B'],
|
||||
connections: { Trigger: ['Node A'], 'Node A': ['Node B'] },
|
||||
}),
|
||||
);
|
||||
|
||||
await hooks.runHook('nodeExecuteBefore', ['Trigger', taskStartedData]);
|
||||
await hooks.runHook('nodeExecuteBefore', ['Node A', taskStartedData]);
|
||||
expect(push.send).not.toHaveBeenCalled();
|
||||
|
||||
stubActiveExecution(parentExecutionId, { pushRef: rootPushRef });
|
||||
await hooks.runHook('nodeExecuteBefore', ['Node B', taskStartedData]);
|
||||
|
||||
expect(push.send).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
type: 'subworkflowNodeProgress',
|
||||
data: expect.objectContaining({ currentNodeName: 'Node B', currentNodeIndex: 3 }),
|
||||
}),
|
||||
rootPushRef,
|
||||
);
|
||||
});
|
||||
|
||||
it('excludes disabled nodes and sticky notes from totalNodes', async () => {
|
||||
stubActiveExecution(parentExecutionId, { pushRef: rootPushRef });
|
||||
const activeNode = workflowData.nodes[0];
|
||||
|
|
|
|||
|
|
@ -552,7 +552,11 @@ function hookFunctionsPushSubExecution(
|
|||
parentExecution: RelatedExecution,
|
||||
parentNode: INode,
|
||||
) {
|
||||
const totalNodes = countReachableNodes(workflowData);
|
||||
// Deferred until something is actually watching. Most executions have no
|
||||
// editor session, and a parent loop can spawn hundreds of sub-executions, so
|
||||
// the graph traversal must not run just because hooks were registered.
|
||||
let cachedTotalNodes: number | undefined;
|
||||
const getTotalNodes = (): number => (cachedTotalNodes ??= countReachableNodes(workflowData));
|
||||
|
||||
// Push + pushRef are resolved lazily on first emit. This keeps hook
|
||||
// registration free of DI side effects (cli tests globally `jest.mock`
|
||||
|
|
@ -580,12 +584,11 @@ function hookFunctionsPushSubExecution(
|
|||
|
||||
// Trailing-edge throttle. The engine emits a before/after pair per node
|
||||
// execution, so a looping child produces two events per iteration — each of
|
||||
// which is a pubsub broadcast in scaling mode and a full canvas re-map in
|
||||
// the editor. The overlay only renders "Running: X", "i / N" and a bar, so
|
||||
// intermediate states are disposable: we coalesce to at most one message
|
||||
// per window and always send the latest state, which caps push volume
|
||||
// regardless of how fast or how long the child runs.
|
||||
const throttleMs = totalNodes >= 50 ? 250 : 100;
|
||||
// which is a pubsub broadcast in scaling mode and a canvas re-map in the
|
||||
// editor. Only the latest state is ever rendered, so intermediate ones are
|
||||
// disposable: we coalesce to at most one message per window, which caps push
|
||||
// volume regardless of how fast or how long the child runs.
|
||||
const getThrottleMs = () => (getTotalNodes() >= 50 ? 250 : 100);
|
||||
let lastEmitAt = 0;
|
||||
let pending: PushPayload<'subworkflowNodeProgress'> | undefined;
|
||||
let timer: NodeJS.Timeout | undefined;
|
||||
|
|
@ -619,7 +622,15 @@ function hookFunctionsPushSubExecution(
|
|||
phase: 'running' | 'success' | 'error',
|
||||
childId: string,
|
||||
) {
|
||||
// Counted before the bail-out, so the tally stays correct if an editor
|
||||
// session shows up partway through the run.
|
||||
reachedNodeNames.add(nodeName);
|
||||
|
||||
// Nothing is watching (production run, or the parent isn't registered yet).
|
||||
// Resolution is retried on the next event, so this doesn't silence the
|
||||
// stream — it just keeps the whole overlay off the hot path.
|
||||
if (!resolveTarget()) return;
|
||||
|
||||
pending = {
|
||||
parentExecutionId: parentExecution.executionId,
|
||||
parentNodeName: parentNode.name,
|
||||
|
|
@ -629,9 +640,10 @@ function hookFunctionsPushSubExecution(
|
|||
// estimated, so clamping to it would spoil the one exact number here. The
|
||||
// editor shows this as a plain count and clamps the arc separately.
|
||||
currentNodeIndex: reachedNodeNames.size,
|
||||
totalNodes,
|
||||
totalNodes: getTotalNodes(),
|
||||
phase,
|
||||
};
|
||||
const throttleMs = getThrottleMs();
|
||||
const elapsed = Date.now() - lastEmitAt;
|
||||
// Leading edge: the window has passed, so emit straight away.
|
||||
if (elapsed >= throttleMs) {
|
||||
|
|
@ -655,7 +667,7 @@ function hookFunctionsPushSubExecution(
|
|||
parentExecutionId: parentExecution.executionId,
|
||||
parentNodeName: parentNode.name,
|
||||
executionId: this.executionId,
|
||||
totalNodes,
|
||||
totalNodes: getTotalNodes(),
|
||||
},
|
||||
},
|
||||
target.pushRef,
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ import type { CanvasNodeDefaultRender } from '../../../../canvas.types';
|
|||
import { injectCanvasRenderData } from '@/features/workflows/canvas/canvas.utils';
|
||||
import { useCanvas } from '../../../../composables/useCanvas';
|
||||
import { useZoomAdjustedValues } from '../../../../composables/useZoomAdjustedValues';
|
||||
import { useSubworkflowProgressArc } from '../../../../composables/useSubworkflowProgressArc';
|
||||
import CanvasNodeSettingsIcons from './parts/CanvasNodeSettingsIcons.vue';
|
||||
import { useNodePrivateCredential } from '@/features/resolvers/composables/useNodePrivateCredential';
|
||||
import { useNodeHelpers } from '@/app/composables/useNodeHelpers';
|
||||
|
|
@ -142,14 +141,18 @@ const displaySubtitle = computed(() => {
|
|||
});
|
||||
});
|
||||
|
||||
const { fraction: subworkflowProgressFraction } = useSubworkflowProgressArc(subworkflowProgress);
|
||||
/** The total is only an upper bound, so a full turn is reserved for completion. */
|
||||
const MAX_PROGRESS_FRACTION = 0.9;
|
||||
|
||||
const subworkflowProgressStyle = computed(() => {
|
||||
if (subworkflowProgressFraction.value <= 0) return {};
|
||||
const progress = subworkflowProgress.value;
|
||||
if (!progress || progress.totalNodes <= 0) return {};
|
||||
|
||||
return {
|
||||
'--node--progress--fraction': `${(subworkflowProgressFraction.value * 100).toFixed(2)}%`,
|
||||
};
|
||||
const fraction = Math.min(
|
||||
Math.max(progress.currentNodeIndex / progress.totalNodes, 0),
|
||||
MAX_PROGRESS_FRACTION,
|
||||
);
|
||||
return { '--node--progress--fraction': `${(fraction * 100).toFixed(2)}%` };
|
||||
});
|
||||
|
||||
const styles = computed(() => ({
|
||||
|
|
|
|||
|
|
@ -1,105 +0,0 @@
|
|||
import { effectScope, ref } from 'vue';
|
||||
import { useSubworkflowProgressArc } from './useSubworkflowProgressArc';
|
||||
|
||||
type Progress = { currentNodeIndex: number; totalNodes: number } | undefined;
|
||||
|
||||
/**
|
||||
* `useIntervalFn` disposes with its effect scope, so each case runs inside its
|
||||
* own and stops it afterwards — otherwise timers leak across tests and the
|
||||
* trickle from one case advances the next.
|
||||
*/
|
||||
function withArc(initial: Progress) {
|
||||
const progress = ref<Progress>(initial);
|
||||
const scope = effectScope();
|
||||
const arc = scope.run(() => useSubworkflowProgressArc(progress))!;
|
||||
|
||||
return { progress, fraction: arc.fraction, stop: () => scope.stop() };
|
||||
}
|
||||
|
||||
describe('useSubworkflowProgressArc', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('reports 0 without progress, so the caller can skip the arc', () => {
|
||||
const { fraction, stop } = withArc(undefined);
|
||||
|
||||
expect(fraction.value).toBe(0);
|
||||
stop();
|
||||
});
|
||||
|
||||
it('reports 0 when the total is unknown', () => {
|
||||
const { fraction, stop } = withArc({ currentNodeIndex: 2, totalNodes: 0 });
|
||||
|
||||
expect(fraction.value).toBe(0);
|
||||
stop();
|
||||
});
|
||||
|
||||
it('moves to the step fraction as soon as a step lands', async () => {
|
||||
const { progress, fraction, stop } = withArc({ currentNodeIndex: 1, totalNodes: 4 });
|
||||
|
||||
progress.value = { currentNodeIndex: 2, totalNodes: 4 };
|
||||
await vi.waitFor(() => expect(fraction.value).toBe(0.5));
|
||||
stop();
|
||||
});
|
||||
|
||||
it('trickles forward between steps without reaching the next one', async () => {
|
||||
const { fraction, stop } = withArc({ currentNodeIndex: 2, totalNodes: 4 });
|
||||
expect(fraction.value).toBe(0.5);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(60_000);
|
||||
|
||||
// Bounded by half a step: 0.5 + (1/4 * 0.5) = 0.625, approached but never met.
|
||||
expect(fraction.value).toBeGreaterThan(0.5);
|
||||
expect(fraction.value).toBeLessThan(0.625);
|
||||
stop();
|
||||
});
|
||||
|
||||
it('never exceeds the cap, however long a step runs', async () => {
|
||||
const { fraction, stop } = withArc({ currentNodeIndex: 4, totalNodes: 4 });
|
||||
|
||||
await vi.advanceTimersByTimeAsync(600_000);
|
||||
|
||||
expect(fraction.value).toBeLessThanOrEqual(0.9);
|
||||
stop();
|
||||
});
|
||||
|
||||
it('does not move backwards when a step lands behind the trickle', async () => {
|
||||
const { progress, fraction, stop } = withArc({ currentNodeIndex: 2, totalNodes: 4 });
|
||||
|
||||
await vi.advanceTimersByTimeAsync(30_000);
|
||||
const trickled = fraction.value;
|
||||
expect(trickled).toBeGreaterThan(0.5);
|
||||
|
||||
// Step 3 of 4 lands at 0.75, ahead of the trickle — but assert the general
|
||||
// rule: a real step is never allowed to regress the arc.
|
||||
progress.value = { currentNodeIndex: 3, totalNodes: 4 };
|
||||
await vi.waitFor(() => expect(fraction.value).toBeGreaterThanOrEqual(trickled));
|
||||
stop();
|
||||
});
|
||||
|
||||
it('snaps back when a new child execution restarts the count', async () => {
|
||||
const { progress, fraction, stop } = withArc({ currentNodeIndex: 3, totalNodes: 4 });
|
||||
expect(fraction.value).toBe(0.75);
|
||||
|
||||
// A parent loop invoking the sub-workflow again: the arc must restart, not
|
||||
// hold the previous run's high-water mark.
|
||||
progress.value = { currentNodeIndex: 1, totalNodes: 4 };
|
||||
await vi.waitFor(() => expect(fraction.value).toBe(0.25));
|
||||
stop();
|
||||
});
|
||||
|
||||
it('stops trickling once its scope is disposed', async () => {
|
||||
const { fraction, stop } = withArc({ currentNodeIndex: 2, totalNodes: 4 });
|
||||
|
||||
stop();
|
||||
const settled = fraction.value;
|
||||
await vi.advanceTimersByTimeAsync(60_000);
|
||||
|
||||
expect(fraction.value).toBe(settled);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,64 +0,0 @@
|
|||
import { computed, ref, toValue, watch } from 'vue';
|
||||
import type { MaybeRefOrGetter, Ref } from 'vue';
|
||||
import { useIntervalFn } from '@vueuse/core';
|
||||
|
||||
/** The total is only an upper bound, so a full turn is reserved for completion. */
|
||||
const MAX_FRACTION = 0.9;
|
||||
|
||||
/** Share of the gap closed per tick — an exponential approach, so it decelerates. */
|
||||
const TRICKLE_RATE = 0.15;
|
||||
|
||||
const TRICKLE_INTERVAL_MS = 1000;
|
||||
|
||||
/** Half a step: bounds the overstatement to less than one node, by construction. */
|
||||
const TRICKLE_STEP_SHARE = 0.5;
|
||||
|
||||
type Progress = { currentNodeIndex: number; totalNodes: number } | undefined;
|
||||
|
||||
/**
|
||||
* Eased fill fraction for the sub-workflow progress arc, in [0, MAX_FRACTION].
|
||||
* Steps move it immediately; between them it trickles so a slow child node
|
||||
* doesn't read as a stalled bar. Returns 0 when there's nothing to draw.
|
||||
*/
|
||||
export function useSubworkflowProgressArc(progress: MaybeRefOrGetter<Progress>): {
|
||||
fraction: Ref<number>;
|
||||
} {
|
||||
/** Fraction implied by completed steps alone — the part we actually know. */
|
||||
const stepFraction = computed(() => {
|
||||
const value = toValue(progress);
|
||||
if (!value || value.totalNodes <= 0) return 0;
|
||||
|
||||
return Math.min(Math.max(value.currentNodeIndex / value.totalNodes, 0), MAX_FRACTION);
|
||||
});
|
||||
|
||||
/** Ceiling the trickle may approach: this step plus part of the next. */
|
||||
const trickleTarget = computed(() => {
|
||||
const value = toValue(progress);
|
||||
if (!value || value.totalNodes <= 0) return 0;
|
||||
|
||||
const stepSize = 1 / value.totalNodes;
|
||||
return Math.min(stepFraction.value + stepSize * TRICKLE_STEP_SHARE, MAX_FRACTION);
|
||||
});
|
||||
|
||||
const fraction = ref(stepFraction.value);
|
||||
|
||||
watch(stepFraction, (next, previous) => {
|
||||
// A drop means a new child execution restarted the count (a parent loop
|
||||
// calling the sub-workflow repeatedly), so don't hold the old high-water mark.
|
||||
if (next < previous) {
|
||||
fraction.value = next;
|
||||
return;
|
||||
}
|
||||
|
||||
fraction.value = Math.max(fraction.value, next);
|
||||
});
|
||||
|
||||
useIntervalFn(() => {
|
||||
const target = trickleTarget.value;
|
||||
if (fraction.value >= target) return;
|
||||
|
||||
fraction.value += (target - fraction.value) * TRICKLE_RATE;
|
||||
}, TRICKLE_INTERVAL_MS);
|
||||
|
||||
return { fraction };
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user