From 0adc53371969ac3f759d06d9fbb095267fffe95e Mon Sep 17 00:00:00 2001 From: Val <68596159+valya@users.noreply.github.com> Date: Wed, 4 Oct 2023 13:57:21 +0100 Subject: [PATCH] feat: Workflow History pruning and prune time settings (#7343) Github issue / Community forum post (link here to close automatically): --- packages/cli/src/License.ts | 6 + packages/cli/src/Server.ts | 17 +- packages/cli/src/commands/BaseCommand.ts | 5 + packages/cli/src/commands/start.ts | 1 + packages/cli/src/config/schema.ts | 16 ++ packages/cli/src/constants.ts | 1 + .../workflows/workflowHistory/constants.ts | 3 + .../workflowHistoryHelper.ee.ts | 26 ++- .../workflowHistoryManager.ee.ts | 45 ++++ .../cli/test/integration/shared/testDb.ts | 16 ++ .../workflowHistoryManager.test.ts | 208 ++++++++++++++++++ .../test/unit/workflowHistoryHelper.test.ts | 55 +++++ packages/workflow/src/Interfaces.ts | 4 + 13 files changed, 401 insertions(+), 2 deletions(-) create mode 100644 packages/cli/src/workflows/workflowHistory/constants.ts create mode 100644 packages/cli/src/workflows/workflowHistory/workflowHistoryManager.ee.ts create mode 100644 packages/cli/test/integration/workflowHistoryManager.test.ts create mode 100644 packages/cli/test/unit/workflowHistoryHelper.test.ts diff --git a/packages/cli/src/License.ts b/packages/cli/src/License.ts index 672a8959e1..feec7c5c5b 100644 --- a/packages/cli/src/License.ts +++ b/packages/cli/src/License.ts @@ -244,6 +244,12 @@ export class License { return this.getFeatureValue(LICENSE_QUOTAS.VARIABLES_LIMIT) ?? UNLIMITED_LICENSE_QUOTA; } + getWorkflowHistoryPruneLimit() { + return ( + this.getFeatureValue(LICENSE_QUOTAS.WORKFLOW_HISTORY_PRUNE_LIMIT) ?? UNLIMITED_LICENSE_QUOTA + ); + } + getPlanName(): string { return this.getFeatureValue('planName') ?? 'Community'; } diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 93a6128dfa..a6197c50e9 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -178,7 +178,11 @@ import { JwtService } from './services/jwt.service'; import { RoleService } from './services/role.service'; import { UserService } from './services/user.service'; import { OrchestrationController } from './controllers/orchestration.controller'; -import { isWorkflowHistoryEnabled } from './workflows/workflowHistory/workflowHistoryHelper.ee'; +import { + getWorkflowHistoryLicensePruneTime, + getWorkflowHistoryPruneTime, + isWorkflowHistoryEnabled, +} from './workflows/workflowHistory/workflowHistoryHelper.ee'; import { WorkflowHistoryController } from './workflows/workflowHistory/workflowHistory.controller.ee'; const exec = promisify(callbackExec); @@ -350,6 +354,10 @@ export class Server extends AbstractServer { ai: { enabled: config.getEnv('ai.enabled'), }, + workflowHistory: { + pruneTime: -1, + licensePruneTime: -1, + }, }; } @@ -496,6 +504,13 @@ export class Server extends AbstractServer { this.frontendSettings.variables.limit = getVariablesLimit(); } + if (isWorkflowHistoryEnabled()) { + Object.assign(this.frontendSettings.workflowHistory, { + pruneTime: getWorkflowHistoryPruneTime(), + licensePruneTime: getWorkflowHistoryLicensePruneTime(), + }); + } + if (config.get('nodes.packagesMissing').length > 0) { this.frontendSettings.missingPackages = true; } diff --git a/packages/cli/src/commands/BaseCommand.ts b/packages/cli/src/commands/BaseCommand.ts index 764ee0c31f..49072a1082 100644 --- a/packages/cli/src/commands/BaseCommand.ts +++ b/packages/cli/src/commands/BaseCommand.ts @@ -23,6 +23,7 @@ import { License } from '@/License'; import { ExternalSecretsManager } from '@/ExternalSecrets/ExternalSecretsManager.ee'; import { initExpressionEvaluator } from '@/ExpressionEvalator'; import { generateHostInstanceId } from '../databases/utils/generators'; +import { WorkflowHistoryManager } from '@/workflows/workflowHistory/workflowHistoryManager.ee'; export abstract class BaseCommand extends Command { protected logger = LoggerProxy.init(getLogger()); @@ -161,6 +162,10 @@ export abstract class BaseCommand extends Command { await secretsManager.init(); } + initWorkflowHistory() { + Container.get(WorkflowHistoryManager).init(); + } + async finally(error: Error | undefined) { if (inTest || this.id === 'start') return; if (Db.connectionState.connected) { diff --git a/packages/cli/src/commands/start.ts b/packages/cli/src/commands/start.ts index 676f8844fd..1ddf804bae 100644 --- a/packages/cli/src/commands/start.ts +++ b/packages/cli/src/commands/start.ts @@ -216,6 +216,7 @@ export class Start extends BaseCommand { await this.initBinaryDataService(); await this.initExternalHooks(); await this.initExternalSecrets(); + this.initWorkflowHistory(); if (!config.getEnv('endpoints.disableUi')) { await this.generateStaticAssets(); diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 4e7420cf5c..0abcea0c80 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -1227,4 +1227,20 @@ export const schema = { env: 'N8N_SOURCECONTROL_DEFAULT_SSH_KEY_TYPE', }, }, + + workflowHistory: { + enabled: { + doc: 'Whether to save workflow history versions', + format: Boolean, + default: true, + env: 'N8N_WORKFLOW_HISTORY_ENABLED', + }, + + pruneTime: { + doc: 'Time (in hours) to keep workflow history versions for', + format: Number, + default: -1, + env: 'N8N_WORKFLOW_HISTORY_PRUNE_TIME', + }, + }, }; diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts index c94af1f5c6..6c6834409e 100644 --- a/packages/cli/src/constants.ts +++ b/packages/cli/src/constants.ts @@ -87,6 +87,7 @@ export const LICENSE_QUOTAS = { TRIGGER_LIMIT: 'quota:activeWorkflows', VARIABLES_LIMIT: 'quota:maxVariables', USERS_LIMIT: 'quota:users', + WORKFLOW_HISTORY_PRUNE_LIMIT: 'quota:workflowHistoryPrune', } as const; export const UNLIMITED_LICENSE_QUOTA = -1; diff --git a/packages/cli/src/workflows/workflowHistory/constants.ts b/packages/cli/src/workflows/workflowHistory/constants.ts new file mode 100644 index 0000000000..dc4f7c7867 --- /dev/null +++ b/packages/cli/src/workflows/workflowHistory/constants.ts @@ -0,0 +1,3 @@ +import { TIME } from '@/constants'; + +export const WORKFLOW_HISTORY_PRUNE_INTERVAL = 1 * TIME.HOUR; diff --git a/packages/cli/src/workflows/workflowHistory/workflowHistoryHelper.ee.ts b/packages/cli/src/workflows/workflowHistory/workflowHistoryHelper.ee.ts index 49d80b2daa..bf784c5aba 100644 --- a/packages/cli/src/workflows/workflowHistory/workflowHistoryHelper.ee.ts +++ b/packages/cli/src/workflows/workflowHistory/workflowHistoryHelper.ee.ts @@ -1,4 +1,5 @@ import { License } from '@/License'; +import config from '@/config'; import Container from 'typedi'; export function isWorkflowHistoryLicensed() { @@ -7,5 +8,28 @@ export function isWorkflowHistoryLicensed() { } export function isWorkflowHistoryEnabled() { - return isWorkflowHistoryLicensed(); + return isWorkflowHistoryLicensed() && config.getEnv('workflowHistory.enabled'); +} + +export function getWorkflowHistoryLicensePruneTime() { + return Container.get(License).getWorkflowHistoryPruneLimit(); +} + +// Time in hours +export function getWorkflowHistoryPruneTime(): number { + const licenseTime = Container.get(License).getWorkflowHistoryPruneLimit(); + const configTime = config.getEnv('workflowHistory.pruneTime'); + + // License is infinite and config time is infinite + if (licenseTime === -1) { + return configTime; + } + + // License is not infinite but config is, use license time + if (configTime === -1) { + return licenseTime; + } + + // Return the smallest of the license or config if not infinite + return Math.min(configTime, licenseTime); } diff --git a/packages/cli/src/workflows/workflowHistory/workflowHistoryManager.ee.ts b/packages/cli/src/workflows/workflowHistory/workflowHistoryManager.ee.ts new file mode 100644 index 0000000000..0fe5de0b24 --- /dev/null +++ b/packages/cli/src/workflows/workflowHistory/workflowHistoryManager.ee.ts @@ -0,0 +1,45 @@ +import { WorkflowHistoryRepository } from '@/databases/repositories'; +import { Service } from 'typedi'; +import { WORKFLOW_HISTORY_PRUNE_INTERVAL } from './constants'; +import { getWorkflowHistoryPruneTime, isWorkflowHistoryEnabled } from './workflowHistoryHelper.ee'; +import { DateTime } from 'luxon'; +import { LessThan } from 'typeorm'; + +@Service() +export class WorkflowHistoryManager { + pruneTimer?: NodeJS.Timeout; + + constructor(private workflowHistoryRepo: WorkflowHistoryRepository) {} + + init() { + if (this.pruneTimer !== undefined) { + clearInterval(this.pruneTimer); + } + + this.pruneTimer = setInterval(async () => this.prune(), WORKFLOW_HISTORY_PRUNE_INTERVAL); + } + + shutdown() { + if (this.pruneTimer !== undefined) { + clearInterval(this.pruneTimer); + this.pruneTimer = undefined; + } + } + + async prune() { + if (!isWorkflowHistoryEnabled()) { + return; + } + + const pruneHours = getWorkflowHistoryPruneTime(); + // No prune time set + if (pruneHours === -1) { + return; + } + const pruneDateTime = DateTime.now().minus({ hours: pruneHours }).toJSDate(); + + await this.workflowHistoryRepo.delete({ + createdAt: LessThan(pruneDateTime), + }); + } +} diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index ee59624881..806be2f2c5 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -610,6 +610,22 @@ export async function createWorkflowHistoryItem( }); } +export async function createManyWorkflowHistoryItems( + workflowId: string, + count: number, + time?: Date, +) { + const baseTime = (time ?? new Date()).valueOf(); + return Promise.all( + [...Array(count)].map(async (_, i) => + createWorkflowHistoryItem(workflowId, { + createdAt: new Date(baseTime + i), + updatedAt: new Date(baseTime + i), + }), + ), + ); +} + // ---------------------------------- // connection options // ---------------------------------- diff --git a/packages/cli/test/integration/workflowHistoryManager.test.ts b/packages/cli/test/integration/workflowHistoryManager.test.ts new file mode 100644 index 0000000000..892b368de8 --- /dev/null +++ b/packages/cli/test/integration/workflowHistoryManager.test.ts @@ -0,0 +1,208 @@ +import { WorkflowHistoryRepository } from '@/databases/repositories'; +import * as testDb from './shared/testDb'; +import { License } from '@/License'; +import { mockInstance } from './shared/utils'; +import { WorkflowHistoryManager } from '@/workflows/workflowHistory/workflowHistoryManager.ee'; +import Container from 'typedi'; +import config from '@/config'; +import { DateTime } from 'luxon'; +import { In } from 'typeorm'; + +let licenseMock: License; +let licensePruneTime = -1; +let licenseEnabled = true; +let manager: WorkflowHistoryManager; + +beforeAll(async () => { + await testDb.init(); + + licenseMock = mockInstance(License, { + isWorkflowHistoryLicensed() { + return licenseEnabled; + }, + getWorkflowHistoryPruneLimit() { + return licensePruneTime; + }, + }); +}); + +beforeEach(async () => { + await testDb.truncate([WorkflowHistoryRepository]); + jest.useRealTimers(); + jest.clearAllMocks(); + config.set('workflowHistory.enabled', true); + config.set('workflowHistory.pruneTime', -1); + licensePruneTime = -1; + licenseEnabled = true; +}); + +afterEach(() => { + manager?.shutdown(); +}); + +describe('Workflow History Manager', () => { + test('should prune on interval', () => { + jest.useFakeTimers(); + + manager = new WorkflowHistoryManager(Container.get(WorkflowHistoryRepository)); + manager.init(); + const pruneSpy = jest.spyOn(manager, 'prune'); + const currentCount = pruneSpy.mock.calls.length; + + jest.runOnlyPendingTimers(); + + expect(pruneSpy).toBeCalledTimes(currentCount + 1); + + jest.runOnlyPendingTimers(); + expect(pruneSpy).toBeCalledTimes(currentCount + 2); + }); + + test('should not prune when not licensed', async () => { + // Set a prune time just to make sure it gets to the delete + config.set('workflowHistory.pruneTime', 24); + + licenseEnabled = false; + + const repo = Container.get(WorkflowHistoryRepository); + manager = new WorkflowHistoryManager(repo); + manager.init(); + + const workflow = await testDb.createWorkflow(); + await testDb.createManyWorkflowHistoryItems( + workflow.id, + 10, + DateTime.now().minus({ days: 2 }).toJSDate(), + ); + + expect(await repo.count()).toBe(10); + + const deleteSpy = jest.spyOn(repo, 'delete'); + await manager.prune(); + expect(deleteSpy).not.toBeCalled(); + expect(await repo.count()).toBe(10); + }); + + test('should not prune when licensed but disabled', async () => { + // Set a prune time just to make sure it gets to the delete + config.set('workflowHistory.pruneTime', 24); + + config.set('workflowHistory.enabled', false); + + const repo = Container.get(WorkflowHistoryRepository); + manager = new WorkflowHistoryManager(repo); + manager.init(); + + const workflow = await testDb.createWorkflow(); + await testDb.createManyWorkflowHistoryItems( + workflow.id, + 10, + DateTime.now().minus({ days: 2 }).toJSDate(), + ); + + expect(await repo.count()).toBe(10); + + const deleteSpy = jest.spyOn(repo, 'delete'); + await manager.prune(); + expect(deleteSpy).not.toBeCalled(); + expect(await repo.count()).toBe(10); + }); + + test('should not prune when both prune times are -1 (infinite)', async () => { + config.set('workflowHistory.pruneTime', -1); + licensePruneTime = -1; + + const repo = Container.get(WorkflowHistoryRepository); + manager = new WorkflowHistoryManager(repo); + manager.init(); + + const workflow = await testDb.createWorkflow(); + await testDb.createManyWorkflowHistoryItems( + workflow.id, + 10, + DateTime.now().minus({ days: 2 }).toJSDate(), + ); + + expect(await repo.count()).toBe(10); + + const deleteSpy = jest.spyOn(repo, 'delete'); + await manager.prune(); + expect(deleteSpy).not.toBeCalled(); + expect(await repo.count()).toBe(10); + }); + + test('should prune when config prune time is not -1 (infinite)', async () => { + config.set('workflowHistory.pruneTime', 24); + licensePruneTime = -1; + + const repo = Container.get(WorkflowHistoryRepository); + manager = new WorkflowHistoryManager(repo); + manager.init(); + + const workflow = await testDb.createWorkflow(); + await testDb.createManyWorkflowHistoryItems( + workflow.id, + 10, + DateTime.now().minus({ days: 2 }).toJSDate(), + ); + + expect(await repo.count()).toBe(10); + + const deleteSpy = jest.spyOn(repo, 'delete'); + await manager.prune(); + expect(deleteSpy).toBeCalled(); + expect(await repo.count()).toBe(0); + }); + + test('should prune when license prune time is not -1 (infinite)', async () => { + config.set('workflowHistory.pruneTime', -1); + licensePruneTime = 24; + + const repo = Container.get(WorkflowHistoryRepository); + manager = new WorkflowHistoryManager(repo); + manager.init(); + + const workflow = await testDb.createWorkflow(); + await testDb.createManyWorkflowHistoryItems( + workflow.id, + 10, + DateTime.now().minus({ days: 2 }).toJSDate(), + ); + + expect(await repo.count()).toBe(10); + + const deleteSpy = jest.spyOn(repo, 'delete'); + await manager.prune(); + expect(deleteSpy).toBeCalled(); + expect(await repo.count()).toBe(0); + }); + + test('should only prune versions older than prune time', async () => { + config.set('workflowHistory.pruneTime', 24); + licensePruneTime = -1; + + const repo = Container.get(WorkflowHistoryRepository); + manager = new WorkflowHistoryManager(repo); + manager.init(); + + const workflow = await testDb.createWorkflow(); + const recentVersions = await testDb.createManyWorkflowHistoryItems(workflow.id, 10); + const oldVersions = await testDb.createManyWorkflowHistoryItems( + workflow.id, + 10, + DateTime.now().minus({ days: 2 }).toJSDate(), + ); + + expect(await repo.count()).toBe(20); + + const deleteSpy = jest.spyOn(repo, 'delete'); + await manager.prune(); + expect(deleteSpy).toBeCalled(); + expect(await repo.count()).toBe(10); + expect( + await repo.count({ where: { versionId: In(recentVersions.map((i) => i.versionId)) } }), + ).toBe(10); + expect( + await repo.count({ where: { versionId: In(oldVersions.map((i) => i.versionId)) } }), + ).toBe(0); + }); +}); diff --git a/packages/cli/test/unit/workflowHistoryHelper.test.ts b/packages/cli/test/unit/workflowHistoryHelper.test.ts new file mode 100644 index 0000000000..a6a701452d --- /dev/null +++ b/packages/cli/test/unit/workflowHistoryHelper.test.ts @@ -0,0 +1,55 @@ +import { License } from '@/License'; +import { mockInstance } from '../integration/shared/utils'; +import config from '@/config'; +import { getWorkflowHistoryPruneTime } from '@/workflows/workflowHistory/workflowHistoryHelper.ee'; + +let licenseMock: License; +let licensePruneTime = -1; + +beforeAll(async () => { + licenseMock = mockInstance(License, { + getWorkflowHistoryPruneLimit() { + return licensePruneTime; + }, + }); +}); + +beforeEach(() => { + licensePruneTime = -1; + config.set('workflowHistory.pruneTime', -1); +}); + +describe('getWorkflowHistoryPruneTime', () => { + test('should return -1 (infinite) if config and license are -1', () => { + licensePruneTime = -1; + config.set('workflowHistory.pruneTime', -1); + + expect(getWorkflowHistoryPruneTime()).toBe(-1); + }); + + test('should return config time if license is infinite and config is not', () => { + licensePruneTime = -1; + config.set('workflowHistory.pruneTime', 24); + + expect(getWorkflowHistoryPruneTime()).toBe(24); + }); + + test('should return license time if config is infinite and license is not', () => { + licensePruneTime = 25; + config.set('workflowHistory.pruneTime', -1); + + expect(getWorkflowHistoryPruneTime()).toBe(25); + }); + + test('should return lowest of config and license time if both are not -1', () => { + licensePruneTime = 26; + config.set('workflowHistory.pruneTime', 100); + + expect(getWorkflowHistoryPruneTime()).toBe(26); + + licensePruneTime = 100; + config.set('workflowHistory.pruneTime', 27); + + expect(getWorkflowHistoryPruneTime()).toBe(27); + }); +}); diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 29e54f6fae..8045be528e 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -2329,6 +2329,10 @@ export interface IN8nUISettings { ai: { enabled: boolean; }; + workflowHistory: { + pruneTime: number; + licensePruneTime: number; + }; } export interface SecretsHelpersBase {