refactor(core): Limit soft-deletions to pruning only (#7469)

Based on customer feedback, we should limit soft deletions to pruning
only, to prevent executions from piling up in very high volume cases.
This commit is contained in:
Iván Ovejero 2023-10-20 15:02:47 +02:00 committed by GitHub
parent 3c0a166f7f
commit 0b42d1aa71
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 54 additions and 23 deletions

View file

@ -31,7 +31,10 @@ export = {
return res.status(404).json({ message: 'Not Found' }); return res.status(404).json({ message: 'Not Found' });
} }
await Container.get(ExecutionRepository).softDelete(execution.id); await Container.get(ExecutionRepository).hardDelete({
workflowId: execution.workflowId as string,
executionId: execution.id,
});
execution.id = id; execution.id = id;

View file

@ -516,7 +516,10 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks {
} }
if (isManualMode && !saveManualExecutions && !fullRunData.waitTill) { if (isManualMode && !saveManualExecutions && !fullRunData.waitTill) {
await Container.get(ExecutionRepository).softDelete(this.executionId); await Container.get(ExecutionRepository).hardDelete({
workflowId: this.workflowData.id as string,
executionId: this.executionId,
});
return; return;
} }
@ -547,7 +550,10 @@ function hookFunctionsSave(parentProcessMode?: string): IWorkflowExecuteHooks {
this.executionId, this.executionId,
this.retryOf, this.retryOf,
); );
await Container.get(ExecutionRepository).softDelete(this.executionId); await Container.get(ExecutionRepository).hardDelete({
workflowId: this.workflowData.id as string,
executionId: this.executionId,
});
return; return;
} }

View file

@ -645,7 +645,10 @@ export class WorkflowRunner {
(workflowDidSucceed && saveDataSuccessExecution === 'none') || (workflowDidSucceed && saveDataSuccessExecution === 'none') ||
(!workflowDidSucceed && saveDataErrorExecution === 'none') (!workflowDidSucceed && saveDataErrorExecution === 'none')
) { ) {
await Container.get(ExecutionRepository).softDelete(executionId); await Container.get(ExecutionRepository).hardDelete({
workflowId: data.workflowData.id as string,
executionId,
});
} }
// eslint-disable-next-line id-denylist // eslint-disable-next-line id-denylist
} catch (err) { } catch (err) {

View file

@ -125,7 +125,10 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
} min`, } min`,
); );
this.intervals.softDeletion = setInterval(async () => this.prune(), this.rates.softDeletion); this.intervals.softDeletion = setInterval(
async () => this.softDeleteOnPruningCycle(),
this.rates.softDeletion,
);
} }
setHardDeletionInterval() { setHardDeletionInterval() {
@ -136,7 +139,7 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
); );
this.intervals.hardDeletion = setInterval( this.intervals.hardDeletion = setInterval(
async () => this.hardDelete(), async () => this.hardDeleteOnPruningCycle(),
this.rates.hardDeletion, this.rates.hardDeletion,
); );
} }
@ -294,6 +297,16 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
); );
} }
/**
* Permanently remove a single execution and its binary data.
*/
async hardDelete(ids: { workflowId: string; executionId: string }) {
return Promise.all([
this.binaryDataService.deleteMany([ids]),
this.delete({ id: ids.executionId }),
]);
}
async updateExistingExecution(executionId: string, execution: Partial<IExecutionResponse>) { async updateExistingExecution(executionId: string, execution: Partial<IExecutionResponse>) {
// Se isolate startedAt because it must be set when the execution starts and should never change. // Se isolate startedAt because it must be set when the execution starts and should never change.
// So we prevent updating it, if it's sent (it usually is and causes problems to executions that // So we prevent updating it, if it's sent (it usually is and causes problems to executions that
@ -467,12 +480,15 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
do { do {
// Delete in batches to avoid "SQLITE_ERROR: Expression tree is too large (maximum depth 1000)" error // Delete in batches to avoid "SQLITE_ERROR: Expression tree is too large (maximum depth 1000)" error
const batch = executionIds.splice(0, this.deletionBatchSize); const batch = executionIds.splice(0, this.deletionBatchSize);
await this.softDelete(batch); await this.delete(batch);
} while (executionIds.length > 0); } while (executionIds.length > 0);
} }
async prune() { /**
Logger.verbose('Soft-deleting (pruning) execution data from database'); * Mark executions as deleted based on age and count, in a pruning cycle.
*/
async softDeleteOnPruningCycle() {
Logger.verbose('Soft-deleting execution data from database (pruning cycle)');
const maxAge = config.getEnv('executions.pruneDataMaxAge'); // in h const maxAge = config.getEnv('executions.pruneDataMaxAge'); // in h
const maxCount = config.getEnv('executions.pruneDataMaxCount'); const maxCount = config.getEnv('executions.pruneDataMaxCount');
@ -520,9 +536,9 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
} }
/** /**
* Permanently delete all soft-deleted executions and their binary data, in batches. * Permanently remove all soft-deleted executions and their binary data, in a pruning cycle.
*/ */
private async hardDelete() { private async hardDeleteOnPruningCycle() {
const date = new Date(); const date = new Date();
date.setHours(date.getHours() - config.getEnv('executions.pruneDataHardDeleteBuffer')); date.setHours(date.getHours() - config.getEnv('executions.pruneDataHardDeleteBuffer'));
@ -551,9 +567,12 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
await this.binaryDataService.deleteMany(workflowIdsAndExecutionIds); await this.binaryDataService.deleteMany(workflowIdsAndExecutionIds);
this.logger.debug(`Hard-deleting ${executionIds.length} executions from database`, { this.logger.debug(
executionIds, `Hard-deleting ${executionIds.length} executions from database (pruning cycle)`,
}); {
executionIds,
},
);
// Actually delete these executions // Actually delete these executions
await this.delete({ id: In(executionIds) }); await this.delete({ id: In(executionIds) });
@ -569,7 +588,7 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
if (executionIds.length === this.deletionBatchSize) { if (executionIds.length === this.deletionBatchSize) {
clearInterval(this.intervals.hardDeletion); clearInterval(this.intervals.hardDeletion);
setTimeout(async () => this.hardDelete(), 1 * TIME.SECOND); setTimeout(async () => this.hardDeleteOnPruningCycle(), 1 * TIME.SECOND);
} else { } else {
if (this.intervals.hardDeletion) return; if (this.intervals.hardDeletion) return;

View file

@ -57,7 +57,7 @@ describe('ExecutionRepository.prune()', () => {
await testDb.createSuccessfulExecution(workflow), await testDb.createSuccessfulExecution(workflow),
]; ];
await executionRepository.prune(); await executionRepository.softDeleteOnPruningCycle();
const result = await findAllExecutions(); const result = await findAllExecutions();
expect(result).toEqual([ expect(result).toEqual([
@ -76,7 +76,7 @@ describe('ExecutionRepository.prune()', () => {
await testDb.createSuccessfulExecution(workflow), await testDb.createSuccessfulExecution(workflow),
]; ];
await executionRepository.prune(); await executionRepository.softDeleteOnPruningCycle();
const result = await findAllExecutions(); const result = await findAllExecutions();
expect(result).toEqual([ expect(result).toEqual([
@ -98,7 +98,7 @@ describe('ExecutionRepository.prune()', () => {
await testDb.createSuccessfulExecution(workflow), await testDb.createSuccessfulExecution(workflow),
]; ];
await executionRepository.prune(); await executionRepository.softDeleteOnPruningCycle();
const result = await findAllExecutions(); const result = await findAllExecutions();
expect(result).toEqual([ expect(result).toEqual([
@ -117,7 +117,7 @@ describe('ExecutionRepository.prune()', () => {
await testDb.createSuccessfulExecution(workflow), await testDb.createSuccessfulExecution(workflow),
]; ];
await executionRepository.prune(); await executionRepository.softDeleteOnPruningCycle();
const result = await findAllExecutions(); const result = await findAllExecutions();
expect(result).toEqual([ expect(result).toEqual([
@ -145,7 +145,7 @@ describe('ExecutionRepository.prune()', () => {
), ),
]; ];
await executionRepository.prune(); await executionRepository.softDeleteOnPruningCycle();
const result = await findAllExecutions(); const result = await findAllExecutions();
expect(result).toEqual([ expect(result).toEqual([
@ -169,7 +169,7 @@ describe('ExecutionRepository.prune()', () => {
await testDb.createSuccessfulExecution(workflow), await testDb.createSuccessfulExecution(workflow),
]; ];
await executionRepository.prune(); await executionRepository.softDeleteOnPruningCycle();
const result = await findAllExecutions(); const result = await findAllExecutions();
expect(result).toEqual([ expect(result).toEqual([
@ -188,7 +188,7 @@ describe('ExecutionRepository.prune()', () => {
])('should prune %s executions', async (status, attributes) => { ])('should prune %s executions', async (status, attributes) => {
const execution = await testDb.createExecution({ status, ...attributes }, workflow); const execution = await testDb.createExecution({ status, ...attributes }, workflow);
await executionRepository.prune(); await executionRepository.softDeleteOnPruningCycle();
const result = await findAllExecutions(); const result = await findAllExecutions();
expect(result).toEqual([ expect(result).toEqual([
@ -206,7 +206,7 @@ describe('ExecutionRepository.prune()', () => {
await testDb.createSuccessfulExecution(workflow), await testDb.createSuccessfulExecution(workflow),
]; ];
await executionRepository.prune(); await executionRepository.softDeleteOnPruningCycle();
const result = await findAllExecutions(); const result = await findAllExecutions();
expect(result).toEqual([ expect(result).toEqual([