Skip to content

Commit

Permalink
test(core): Expand test coverage for pruning (#11567)
Browse files Browse the repository at this point in the history
  • Loading branch information
ivov authored Nov 5, 2024
1 parent 46eceab commit 19d55da
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 27 deletions.
1 change: 1 addition & 0 deletions packages/@n8n/config/src/configs/logging.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export const LOG_SCOPES = [
'external-secrets',
'license',
'multi-main-setup',
'pruning',
'pubsub',
'redis',
'scaling',
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { Subscriber } from '@/scaling/pubsub/subscriber.service';
import { Server } from '@/server';
import { OrchestrationService } from '@/services/orchestration.service';
import { OwnershipService } from '@/services/ownership.service';
import { PruningService } from '@/services/pruning.service';
import { PruningService } from '@/services/pruning/pruning.service';
import { UrlService } from '@/services/url.service';
import { WaitTracker } from '@/wait-tracker';
import { WorkflowRunner } from '@/workflow-runner';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
.execute();
}

async hardDeleteSoftDeletedExecutions() {
async findSoftDeletedExecutions() {
const date = new Date();
date.setHours(date.getHours() - this.globalConfig.pruning.hardDeleteBuffer);

Expand Down
213 changes: 213 additions & 0 deletions packages/cli/src/services/pruning/__tests__/pruning.service.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
import type { GlobalConfig } from '@n8n/config';
import { mock } from 'jest-mock-extended';
import type { InstanceSettings } from 'n8n-core';

import type { MultiMainSetup } from '@/scaling/multi-main-setup.ee';
import type { OrchestrationService } from '@/services/orchestration.service';
import { mockLogger } from '@test/mocking';

import { PruningService } from '../pruning.service';

describe('PruningService', () => {
describe('init', () => {
it('should start pruning if leader', () => {
const pruningService = new PruningService(
mockLogger(),
mock<InstanceSettings>({ isLeader: true }),
mock(),
mock(),
mock<OrchestrationService>({
isMultiMainSetupEnabled: true,
multiMainSetup: mock<MultiMainSetup>(),
}),
mock(),
);
const startPruningSpy = jest.spyOn(pruningService, 'startPruning');

pruningService.init();

expect(startPruningSpy).toHaveBeenCalled();
});

it('should not start pruning if follower', () => {
const pruningService = new PruningService(
mockLogger(),
mock<InstanceSettings>({ isLeader: false }),
mock(),
mock(),
mock<OrchestrationService>({
isMultiMainSetupEnabled: true,
multiMainSetup: mock<MultiMainSetup>(),
}),
mock(),
);
const startPruningSpy = jest.spyOn(pruningService, 'startPruning');

pruningService.init();

expect(startPruningSpy).not.toHaveBeenCalled();
});

it('should register leadership events if multi-main setup is enabled', () => {
const pruningService = new PruningService(
mockLogger(),
mock<InstanceSettings>({ isLeader: true }),
mock(),
mock(),
mock<OrchestrationService>({
isMultiMainSetupEnabled: true,
multiMainSetup: mock<MultiMainSetup>({ on: jest.fn() }),
}),
mock(),
);

pruningService.init();

// @ts-expect-error Private method
expect(pruningService.orchestrationService.multiMainSetup.on).toHaveBeenCalledWith(
'leader-takeover',
expect.any(Function),
);

// @ts-expect-error Private method
expect(pruningService.orchestrationService.multiMainSetup.on).toHaveBeenCalledWith(
'leader-stepdown',
expect.any(Function),
);
});
});

describe('isEnabled', () => {
it('should return `true` based on config if leader main', () => {
const pruningService = new PruningService(
mockLogger(),
mock<InstanceSettings>({ isLeader: true, instanceType: 'main' }),
mock(),
mock(),
mock<OrchestrationService>({
isMultiMainSetupEnabled: true,
multiMainSetup: mock<MultiMainSetup>(),
}),
mock<GlobalConfig>({ pruning: { isEnabled: true } }),
);

// @ts-expect-error Private method
const isEnabled = pruningService.isEnabled();

expect(isEnabled).toBe(true);
});

it('should return `false` based on config if leader main', () => {
const pruningService = new PruningService(
mockLogger(),
mock<InstanceSettings>({ isLeader: true, instanceType: 'main' }),
mock(),
mock(),
mock<OrchestrationService>({
isMultiMainSetupEnabled: true,
multiMainSetup: mock<MultiMainSetup>(),
}),
mock<GlobalConfig>({ pruning: { isEnabled: false } }),
);

// @ts-expect-error Private method
const isEnabled = pruningService.isEnabled();

expect(isEnabled).toBe(false);
});

it('should return `false` if non-main even if enabled', () => {
const pruningService = new PruningService(
mockLogger(),
mock<InstanceSettings>({ isLeader: false, instanceType: 'worker' }),
mock(),
mock(),
mock<OrchestrationService>({
isMultiMainSetupEnabled: true,
multiMainSetup: mock<MultiMainSetup>(),
}),
mock<GlobalConfig>({ pruning: { isEnabled: true } }),
);

// @ts-expect-error Private method
const isEnabled = pruningService.isEnabled();

expect(isEnabled).toBe(false);
});

it('should return `false` if follower main even if enabled', () => {
const pruningService = new PruningService(
mockLogger(),
mock<InstanceSettings>({ isLeader: false, isFollower: true, instanceType: 'main' }),
mock(),
mock(),
mock<OrchestrationService>({
isMultiMainSetupEnabled: true,
multiMainSetup: mock<MultiMainSetup>(),
}),
mock<GlobalConfig>({ pruning: { isEnabled: true }, multiMainSetup: { enabled: true } }),
);

// @ts-expect-error Private method
const isEnabled = pruningService.isEnabled();

expect(isEnabled).toBe(false);
});
});

describe('startPruning', () => {
it('should not start pruning if service is disabled', () => {
const pruningService = new PruningService(
mockLogger(),
mock<InstanceSettings>({ isLeader: true, instanceType: 'main' }),
mock(),
mock(),
mock<OrchestrationService>({
isMultiMainSetupEnabled: true,
multiMainSetup: mock<MultiMainSetup>(),
}),
mock<GlobalConfig>({ pruning: { isEnabled: false } }),
);

// @ts-expect-error Private method
const setSoftDeletionInterval = jest.spyOn(pruningService, 'setSoftDeletionInterval');

// @ts-expect-error Private method
const scheduleHardDeletion = jest.spyOn(pruningService, 'scheduleHardDeletion');

pruningService.startPruning();

expect(setSoftDeletionInterval).not.toHaveBeenCalled();
expect(scheduleHardDeletion).not.toHaveBeenCalled();
});

it('should start pruning if service is enabled', () => {
const pruningService = new PruningService(
mockLogger(),
mock<InstanceSettings>({ isLeader: true, instanceType: 'main' }),
mock(),
mock(),
mock<OrchestrationService>({
isMultiMainSetupEnabled: true,
multiMainSetup: mock<MultiMainSetup>(),
}),
mock<GlobalConfig>({ pruning: { isEnabled: true } }),
);

const setSoftDeletionInterval = jest
// @ts-expect-error Private method
.spyOn(pruningService, 'setSoftDeletionInterval')
.mockImplementation();

const scheduleHardDeletion = jest
// @ts-expect-error Private method
.spyOn(pruningService, 'scheduleHardDeletion')
.mockImplementation();

pruningService.startPruning();

expect(setSoftDeletionInterval).toHaveBeenCalled();
expect(scheduleHardDeletion).toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import { BinaryDataService, InstanceSettings } from 'n8n-core';
import { jsonStringify } from 'n8n-workflow';
import { Service } from 'typedi';

import { inTest, TIME } from '@/constants';
import { TIME } from '@/constants';
import { ExecutionRepository } from '@/databases/repositories/execution.repository';
import { OnShutdown } from '@/decorators/on-shutdown';
import { Logger } from '@/logging/logger.service';

import { OrchestrationService } from './orchestration.service';
import { OrchestrationService } from '../orchestration.service';

@Service()
export class PruningService {
Expand All @@ -32,7 +32,9 @@ export class PruningService {
private readonly binaryDataService: BinaryDataService,
private readonly orchestrationService: OrchestrationService,
private readonly globalConfig: GlobalConfig,
) {}
) {
this.logger = this.logger.scoped('pruning');
}

/**
* @important Requires `OrchestrationService` to be initialized.
Expand All @@ -49,9 +51,9 @@ export class PruningService {
}
}

private isPruningEnabled() {
private isEnabled() {
const { instanceType, isFollower } = this.instanceSettings;
if (!this.globalConfig.pruning.isEnabled || inTest || instanceType !== 'main') {
if (!this.globalConfig.pruning.isEnabled || instanceType !== 'main') {
return false;
}

Expand All @@ -66,23 +68,23 @@ export class PruningService {
* @important Call this method only after DB migrations have completed.
*/
startPruning() {
if (!this.isPruningEnabled()) return;
if (!this.isEnabled()) return;

if (this.isShuttingDown) {
this.logger.warn('[Pruning] Cannot start pruning while shutting down');
this.logger.warn('Cannot start pruning while shutting down');
return;
}

this.logger.debug('[Pruning] Starting soft-deletion and hard-deletion timers');
this.logger.debug('Starting soft-deletion and hard-deletion timers');

this.setSoftDeletionInterval();
this.scheduleHardDeletion();
}

stopPruning() {
if (!this.isPruningEnabled()) return;
if (!this.isEnabled()) return;

this.logger.debug('[Pruning] Removing soft-deletion and hard-deletion timers');
this.logger.debug('Removing soft-deletion and hard-deletion timers');

clearInterval(this.softDeletionInterval);
clearTimeout(this.hardDeletionTimeout);
Expand All @@ -96,7 +98,7 @@ export class PruningService {
this.rates.softDeletion,
);

this.logger.debug(`[Pruning] Soft-deletion scheduled every ${when}`);
this.logger.debug(`Soft-deletion scheduled every ${when}`);
}

private scheduleHardDeletion(rateMs = this.rates.hardDeletion) {
Expand All @@ -113,27 +115,27 @@ export class PruningService {
? error.message
: jsonStringify(error, { replaceCircularRefs: true });

this.logger.error('[Pruning] Failed to hard-delete executions', { errorMessage });
this.logger.error('Failed to hard-delete executions', { errorMessage });
});
}, rateMs);

this.logger.debug(`[Pruning] Hard-deletion scheduled for next ${when}`);
this.logger.debug(`Hard-deletion scheduled for next ${when}`);
}

/**
* Mark executions as deleted based on age and count, in a pruning cycle.
*/
async softDeleteOnPruningCycle() {
this.logger.debug('[Pruning] Starting soft-deletion of executions');
this.logger.debug('Starting soft-deletion of executions');

const result = await this.executionRepository.softDeletePrunableExecutions();

if (result.affected === 0) {
this.logger.debug('[Pruning] Found no executions to soft-delete');
this.logger.debug('Found no executions to soft-delete');
return;
}

this.logger.debug('[Pruning] Soft-deleted executions', { count: result.affected });
this.logger.debug('Soft-deleted executions', { count: result.affected });
}

@OnShutdown()
Expand All @@ -147,26 +149,26 @@ export class PruningService {
* @return Delay in ms after which the next cycle should be started
*/
private async hardDeleteOnPruningCycle() {
const ids = await this.executionRepository.hardDeleteSoftDeletedExecutions();
const ids = await this.executionRepository.findSoftDeletedExecutions();

const executionIds = ids.map((o) => o.executionId);

if (executionIds.length === 0) {
this.logger.debug('[Pruning] Found no executions to hard-delete');
this.logger.debug('Found no executions to hard-delete');

return this.rates.hardDeletion;
}

try {
this.logger.debug('[Pruning] Starting hard-deletion of executions', { executionIds });
this.logger.debug('Starting hard-deletion of executions', { executionIds });

await this.binaryDataService.deleteMany(ids);

await this.executionRepository.deleteByIds(executionIds);

this.logger.debug('[Pruning] Hard-deleted executions', { executionIds });
this.logger.debug('Hard-deleted executions', { executionIds });
} catch (error) {
this.logger.error('[Pruning] Failed to hard-delete executions', {
this.logger.error('Failed to hard-delete executions', {
executionIds,
error: error instanceof Error ? error.message : `${error}`,
});
Expand Down
Loading

0 comments on commit 19d55da

Please sign in to comment.