From 1d870412caba987a502e025d3ae4f6c76209513a 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: Tue, 12 Dec 2023 14:36:56 +0100 Subject: [PATCH] refactor(core): Don't use DB transactions on ExecutionRepository.createNewExecution (#8002) Saving execution data is one of the slowest DB operations in the application, and is likely behind some of the sqlite transaction concurrency issues we've been seeing. This not only remove the 2 separate transactions for saving `ExecutionEntity` and `ExecutionData`, but also remove fields from `ExecutionData.workflowData` that don't need to be saved (like `tags`, `shared`, `statistics`, `triggerCount`, etc). --- packages/cli/src/ActiveExecutions.ts | 4 +- .../repositories/execution.repository.ts | 16 +++--- .../repositories/execution.repository.test.ts | 53 +++++++++++++++++++ .../cli/test/unit/ActiveExecutions.test.ts | 4 +- 4 files changed, 63 insertions(+), 14 deletions(-) create mode 100644 packages/cli/test/integration/database/repositories/execution.repository.test.ts diff --git a/packages/cli/src/ActiveExecutions.ts b/packages/cli/src/ActiveExecutions.ts index ceb74d628c..cd8ce8ecdb 100644 --- a/packages/cli/src/ActiveExecutions.ts +++ b/packages/cli/src/ActiveExecutions.ts @@ -60,9 +60,7 @@ export class ActiveExecutions { fullExecutionData.workflowId = workflowId; } - const executionResult = - await Container.get(ExecutionRepository).createNewExecution(fullExecutionData); - executionId = executionResult.id; + executionId = await Container.get(ExecutionRepository).createNewExecution(fullExecutionData); if (executionId === undefined) { throw new ApplicationError('There was an issue assigning an execution id to the execution'); } diff --git a/packages/cli/src/databases/repositories/execution.repository.ts b/packages/cli/src/databases/repositories/execution.repository.ts index f8e1350588..7598ff788f 100644 --- a/packages/cli/src/databases/repositories/execution.repository.ts +++ b/packages/cli/src/databases/repositories/execution.repository.ts @@ -213,17 +213,17 @@ export class ExecutionRepository extends Repository { return rest; } - async createNewExecution(execution: ExecutionPayload) { + async createNewExecution(execution: ExecutionPayload): Promise { const { data, workflowData, ...rest } = execution; - - const newExecution = await this.save(rest); - await this.executionDataRepository.save({ - execution: newExecution, - workflowData, + const { identifiers: inserted } = await this.insert(rest); + const { id: executionId } = inserted[0] as { id: string }; + const { connections, nodes, name } = workflowData ?? {}; + await this.executionDataRepository.insert({ + executionId, + workflowData: { connections, nodes, name }, data: stringify(data), }); - - return newExecution; + return String(executionId); } async markAsCrashed(executionIds: string[]) { diff --git a/packages/cli/test/integration/database/repositories/execution.repository.test.ts b/packages/cli/test/integration/database/repositories/execution.repository.test.ts new file mode 100644 index 0000000000..d16645367f --- /dev/null +++ b/packages/cli/test/integration/database/repositories/execution.repository.test.ts @@ -0,0 +1,53 @@ +import Container from 'typedi'; +import { ExecutionRepository } from '@db/repositories/execution.repository'; +import { ExecutionDataRepository } from '@db/repositories/executionData.repository'; +import * as testDb from '../../shared/testDb'; +import { createWorkflow } from '../../shared/db/workflows'; + +describe('ExecutionRepository', () => { + beforeAll(async () => { + await testDb.init(); + }); + + beforeEach(async () => { + await testDb.truncate(['Workflow', 'Execution']); + }); + + afterAll(async () => { + await testDb.terminate(); + }); + + describe('createNewExecution', () => { + it('should save execution data', async () => { + const executionRepo = Container.get(ExecutionRepository); + const workflow = await createWorkflow(); + const executionId = await executionRepo.createNewExecution({ + workflowId: workflow.id, + data: { + resultData: {}, + }, + workflowData: workflow, + mode: 'manual', + startedAt: new Date(), + status: 'new', + finished: false, + }); + + expect(executionId).toBeDefined(); + + const executionEntity = await executionRepo.findOneBy({ id: executionId }); + expect(executionEntity?.id).toEqual(executionId); + expect(executionEntity?.workflowId).toEqual(workflow.id); + expect(executionEntity?.status).toEqual('new'); + + const executionDataRepo = Container.get(ExecutionDataRepository); + const executionData = await executionDataRepo.findOneBy({ executionId }); + expect(executionData?.workflowData).toEqual({ + connections: workflow.connections, + nodes: workflow.nodes, + name: workflow.name, + }); + expect(executionData?.data).toEqual('[{"resultData":"1"},{}]'); + }); + }); +}); diff --git a/packages/cli/test/unit/ActiveExecutions.test.ts b/packages/cli/test/unit/ActiveExecutions.test.ts index 689bbbefab..6ac1438b4b 100644 --- a/packages/cli/test/unit/ActiveExecutions.test.ts +++ b/packages/cli/test/unit/ActiveExecutions.test.ts @@ -12,9 +12,7 @@ const FAKE_EXECUTION_ID = '15'; const FAKE_SECOND_EXECUTION_ID = '20'; const updateExistingExecution = jest.fn(); -const createNewExecution = jest.fn(async () => { - return { id: FAKE_EXECUTION_ID }; -}); +const createNewExecution = jest.fn(async () => FAKE_EXECUTION_ID); Container.set(ExecutionRepository, { updateExistingExecution,