From 3428f28a732f79e067b3cb515cc59d835de246a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Thu, 22 Aug 2024 11:46:13 +0200 Subject: [PATCH] fix(core): Scheduler tasks should not trigger on follower instances (#10507) --- packages/core/src/ScheduledTaskManager.ts | 13 ++++++++++++- packages/core/test/ScheduledTaskManager.test.ts | 13 ++++++++++++- .../Schedule/tests/ScheduleTrigger.node.test.ts | 5 +++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/core/src/ScheduledTaskManager.ts b/packages/core/src/ScheduledTaskManager.ts index ce656f3716..eb519a60a7 100644 --- a/packages/core/src/ScheduledTaskManager.ts +++ b/packages/core/src/ScheduledTaskManager.ts @@ -1,13 +1,24 @@ import { Service } from 'typedi'; import { CronJob } from 'cron'; import type { CronExpression, Workflow } from 'n8n-workflow'; +import { InstanceSettings } from './InstanceSettings'; @Service() export class ScheduledTaskManager { + constructor(private readonly instanceSettings: InstanceSettings) {} + readonly cronJobs = new Map(); registerCron(workflow: Workflow, cronExpression: CronExpression, onTick: () => void) { - const cronJob = new CronJob(cronExpression, onTick, undefined, true, workflow.timezone); + const cronJob = new CronJob( + cronExpression, + () => { + if (this.instanceSettings.isLeader) onTick(); + }, + undefined, + true, + workflow.timezone, + ); const cronJobsForWorkflow = this.cronJobs.get(workflow.id); if (cronJobsForWorkflow) { cronJobsForWorkflow.push(cronJob); diff --git a/packages/core/test/ScheduledTaskManager.test.ts b/packages/core/test/ScheduledTaskManager.test.ts index df7fb9b77e..15d5f7d487 100644 --- a/packages/core/test/ScheduledTaskManager.test.ts +++ b/packages/core/test/ScheduledTaskManager.test.ts @@ -1,9 +1,11 @@ import type { Workflow } from 'n8n-workflow'; import { mock } from 'jest-mock-extended'; +import type { InstanceSettings } from '@/InstanceSettings'; import { ScheduledTaskManager } from '@/ScheduledTaskManager'; describe('ScheduledTaskManager', () => { + const instanceSettings = mock({ isLeader: true }); const workflow = mock({ timezone: 'GMT' }); const everyMinute = '0 * * * * *'; const onTick = jest.fn(); @@ -13,7 +15,7 @@ describe('ScheduledTaskManager', () => { beforeEach(() => { jest.clearAllMocks(); jest.useFakeTimers(); - scheduledTaskManager = new ScheduledTaskManager(); + scheduledTaskManager = new ScheduledTaskManager(instanceSettings); }); it('should throw when workflow timezone is invalid', () => { @@ -41,6 +43,15 @@ describe('ScheduledTaskManager', () => { expect(onTick).toHaveBeenCalledTimes(10); }); + it('should should not invoke on follower instances', async () => { + scheduledTaskManager = new ScheduledTaskManager(mock({ isLeader: false })); + scheduledTaskManager.registerCron(workflow, everyMinute, onTick); + + expect(onTick).not.toHaveBeenCalled(); + jest.advanceTimersByTime(10 * 60 * 1000); // 10 minutes + expect(onTick).not.toHaveBeenCalled(); + }); + it('should deregister CronJobs for a workflow', async () => { scheduledTaskManager.registerCron(workflow, everyMinute, onTick); scheduledTaskManager.registerCron(workflow, everyMinute, onTick); diff --git a/packages/nodes-base/nodes/Schedule/tests/ScheduleTrigger.node.test.ts b/packages/nodes-base/nodes/Schedule/tests/ScheduleTrigger.node.test.ts index fa1d2cd615..0693806f4c 100644 --- a/packages/nodes-base/nodes/Schedule/tests/ScheduleTrigger.node.test.ts +++ b/packages/nodes-base/nodes/Schedule/tests/ScheduleTrigger.node.test.ts @@ -1,6 +1,6 @@ import * as n8nWorkflow from 'n8n-workflow'; import type { INode, ITriggerFunctions, Workflow } from 'n8n-workflow'; -import { returnJsonArray } from 'n8n-core'; +import { type InstanceSettings, returnJsonArray } from 'n8n-core'; import { ScheduledTaskManager } from 'n8n-core/dist/ScheduledTaskManager'; import { mock } from 'jest-mock-extended'; import { ScheduleTrigger } from '../ScheduleTrigger.node'; @@ -18,7 +18,8 @@ describe('ScheduleTrigger', () => { const node = mock({ typeVersion: 1 }); const workflow = mock({ timezone }); - const scheduledTaskManager = new ScheduledTaskManager(); + const instanceSettings = mock({ isLeader: true }); + const scheduledTaskManager = new ScheduledTaskManager(instanceSettings); const helpers = mock({ returnJsonArray, registerCron: (cronExpression, onTick) =>