From 6614fa17a519d2f6beb8f3e66b304d5f7412f2b8 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: Wed, 22 May 2024 11:25:29 +0200 Subject: [PATCH 1/9] fix(core): Detect DB connection aquisition deadlocks (no-changelog) use a pool-size of 1 for sqlite-pooled as well --- .github/workflows/ci-postgres-mysql.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-postgres-mysql.yml b/.github/workflows/ci-postgres-mysql.yml index 2277917532d2d..6389da78e9c61 100644 --- a/.github/workflows/ci-postgres-mysql.yml +++ b/.github/workflows/ci-postgres-mysql.yml @@ -42,7 +42,7 @@ jobs: timeout-minutes: 20 env: DB_TYPE: sqlite - DB_SQLITE_POOL_SIZE: 4 + DB_SQLITE_POOL_SIZE: 1 steps: - uses: actions/checkout@v4.1.1 - run: corepack enable @@ -102,6 +102,7 @@ jobs: timeout-minutes: 20 env: DB_POSTGRESDB_PASSWORD: password + DB_POSTGRESDB_POOL_SIZE: 1 # Detect connection pooling deadlocks steps: - uses: actions/checkout@v4.1.1 - run: corepack enable From beea55cb8076bdc0894475057d6ba40a95608155 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: Wed, 22 May 2024 12:43:27 +0200 Subject: [PATCH 2/9] fix UserSubscriber --- .../databases/subscribers/UserSubscriber.ts | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/databases/subscribers/UserSubscriber.ts b/packages/cli/src/databases/subscribers/UserSubscriber.ts index e5fad5bf53698..b925965a0c912 100644 --- a/packages/cli/src/databases/subscribers/UserSubscriber.ts +++ b/packages/cli/src/databases/subscribers/UserSubscriber.ts @@ -1,12 +1,12 @@ +import { Container } from 'typedi'; import type { EntitySubscriberInterface, UpdateEvent } from '@n8n/typeorm'; import { EventSubscriber } from '@n8n/typeorm'; -import { User } from '../entities/User'; -import Container from 'typedi'; -import { ProjectRepository } from '../repositories/project.repository'; import { ApplicationError, ErrorReporterProxy } from 'n8n-workflow'; import { Logger } from '@/Logger'; -import { UserRepository } from '../repositories/user.repository'; + import { Project } from '../entities/Project'; +import { User } from '../entities/User'; +import { UserRepository } from '../repositories/user.repository'; @EventSubscriber() export class UserSubscriber implements EntitySubscriberInterface { @@ -27,14 +27,17 @@ export class UserSubscriber implements EntitySubscriberInterface { fields.includes('email') ) { const oldUser = event.databaseEntity; - const name = + const userEntity = newUserData instanceof User - ? newUserData.createPersonalProjectName() - : Container.get(UserRepository).create(newUserData).createPersonalProjectName(); + ? newUserData + : Container.get(UserRepository).create(newUserData); + + const projectName = userEntity.createPersonalProjectName(); - const project = await Container.get(ProjectRepository).getPersonalProjectForUser( - oldUser.id, - ); + const project = await event.manager.findOneBy(Project, { + type: 'personal', + projectRelations: { userId: oldUser.id }, + }); if (!project) { // Since this is benign we're not throwing the exception. We don't @@ -47,7 +50,7 @@ export class UserSubscriber implements EntitySubscriberInterface { return; } - project.name = name; + project.name = projectName; await event.manager.save(Project, project); } From 89a81bf253232f30a93888a2c6ffaf93954f50a2 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: Wed, 22 May 2024 12:55:07 +0200 Subject: [PATCH 3/9] fix another deadlock --- .../src/PublicApi/v1/handlers/workflows/workflows.service.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts index bb7b8bebd262a..d301e61c93966 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.service.ts @@ -7,7 +7,6 @@ import { SharedWorkflow, type WorkflowSharingRole } from '@db/entities/SharedWor import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; import type { Project } from '@/databases/entities/Project'; -import { WorkflowTagMappingRepository } from '@db/repositories/workflowTagMapping.repository'; import { TagRepository } from '@db/repositories/tag.repository'; import { License } from '@/License'; import { WorkflowSharingService } from '@/workflows/workflowSharing.service'; @@ -113,9 +112,7 @@ export async function getWorkflowTags(workflowId: string) { export async function updateTags(workflowId: string, newTags: string[]): Promise { await Db.transaction(async (transactionManager) => { - const oldTags = await Container.get(WorkflowTagMappingRepository).findBy({ - workflowId, - }); + const oldTags = await transactionManager.findBy(WorkflowTagMapping, { workflowId }); if (oldTags.length > 0) { await transactionManager.delete(WorkflowTagMapping, oldTags); } From 3ac2049c51b54d739ea12c201be5d20640e1cf75 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: Wed, 22 May 2024 13:00:04 +0200 Subject: [PATCH 4/9] fix import service deadlock --- packages/cli/src/services/import.service.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/services/import.service.ts b/packages/cli/src/services/import.service.ts index c2226c65b9896..96892e2745273 100644 --- a/packages/cli/src/services/import.service.ts +++ b/packages/cli/src/services/import.service.ts @@ -1,4 +1,4 @@ -import Container, { Service } from 'typedi'; +import { Service } from 'typedi'; import { v4 as uuid } from 'uuid'; import { type INode, type INodeCredentialsDetails } from 'n8n-workflow'; @@ -8,11 +8,11 @@ import { CredentialsRepository } from '@db/repositories/credentials.repository'; import { TagRepository } from '@db/repositories/tag.repository'; import { SharedWorkflow } from '@db/entities/SharedWorkflow'; import { replaceInvalidCredentials } from '@/WorkflowHelpers'; +import { Project } from '@db/entities/Project'; import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { WorkflowTagMapping } from '@db/entities/WorkflowTagMapping'; import type { TagEntity } from '@db/entities/TagEntity'; import type { ICredentialsDb } from '@/Interfaces'; -import { ProjectRepository } from '@/databases/repositories/project.repository'; @Service() export class ImportService { @@ -59,9 +59,7 @@ export class ImportService { const upsertResult = await tx.upsert(WorkflowEntity, workflow, ['id']); const workflowId = upsertResult.identifiers.at(0)?.id as string; - const personalProject = await Container.get(ProjectRepository).findOneByOrFail({ - id: projectId, - }); + const personalProject = await tx.findOneByOrFail(Project, { id: projectId }); // Create relationship if the workflow was inserted instead of updated. if (!exists) { From 016811ccafbf84de589943065147d3367762d7b6 Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Wed, 22 May 2024 12:31:54 +0200 Subject: [PATCH 5/9] fix another deadlock --- packages/cli/src/commands/import/credentials.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/import/credentials.ts b/packages/cli/src/commands/import/credentials.ts index 03e96aa646b89..ed55bf9176d8d 100644 --- a/packages/cli/src/commands/import/credentials.ts +++ b/packages/cli/src/commands/import/credentials.ts @@ -16,6 +16,7 @@ import { UM_FIX_INSTRUCTION } from '@/constants'; import { UserRepository } from '@db/repositories/user.repository'; import { ProjectRepository } from '@/databases/repositories/project.repository'; import type { Project } from '@/databases/entities/Project'; +import { User } from '@/databases/entities/User'; export class ImportCredentialsCommand extends BaseCommand { static description = 'Import credentials'; @@ -244,7 +245,7 @@ export class ImportCredentialsCommand extends BaseCommand { }); if (sharedCredential && sharedCredential.project.type === 'personal') { - const user = await Container.get(UserRepository).findOneByOrFail({ + const user = await this.transactionManager.findOneByOrFail(User, { projectRelations: { role: 'project:personalOwner', projectId: sharedCredential.projectId, From 04ea3a651727c4708d931ee775603bc307a2e1ae Mon Sep 17 00:00:00 2001 From: Danny Martini Date: Wed, 22 May 2024 12:44:50 +0200 Subject: [PATCH 6/9] fix deadlock --- packages/cli/src/credentials/credentials.service.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/credentials/credentials.service.ts b/packages/cli/src/credentials/credentials.service.ts index d23dbf0cc1eb4..8ce9cdb1d1e4e 100644 --- a/packages/cli/src/credentials/credentials.service.ts +++ b/packages/cli/src/credentials/credentials.service.ts @@ -262,7 +262,10 @@ export class CredentialsService { const project = projectId === undefined - ? await this.projectRepository.getPersonalProjectForUserOrFail(user.id) + ? await this.projectRepository.getPersonalProjectForUserOrFail( + user.id, + transactionManager, + ) : await this.projectService.getProjectWithScope( user, projectId, From efab51dfea6ebdd6dcd90e01b3c6539faec4eacb 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: Wed, 22 May 2024 13:16:49 +0200 Subject: [PATCH 7/9] fix a couple more --- packages/cli/src/WaitTracker.ts | 7 ++-- .../cli/src/commands/import/credentials.ts | 35 ++++++++----------- .../repositories/project.repository.ts | 6 ++-- 3 files changed, 20 insertions(+), 28 deletions(-) diff --git a/packages/cli/src/WaitTracker.ts b/packages/cli/src/WaitTracker.ts index ae6dbf9a627ad..067a58e224544 100644 --- a/packages/cli/src/WaitTracker.ts +++ b/packages/cli/src/WaitTracker.ts @@ -3,7 +3,7 @@ import { ErrorReporterProxy as ErrorReporter, WorkflowOperationError, } from 'n8n-workflow'; -import { Container, Service } from 'typedi'; +import { Service } from 'typedi'; import type { ExecutionStopResult, IWorkflowExecutionDataProcess } from '@/Interfaces'; import { WorkflowRunner } from '@/WorkflowRunner'; import { ExecutionRepository } from '@db/repositories/execution.repository'; @@ -137,10 +137,7 @@ export class WaitTracker { fullExecutionData.waitTill = null; fullExecutionData.status = 'canceled'; - await Container.get(ExecutionRepository).updateExistingExecution( - executionId, - fullExecutionData, - ); + await this.executionRepository.updateExistingExecution(executionId, fullExecutionData); return { mode: fullExecutionData.mode, diff --git a/packages/cli/src/commands/import/credentials.ts b/packages/cli/src/commands/import/credentials.ts index ed55bf9176d8d..6c47a25d96d13 100644 --- a/packages/cli/src/commands/import/credentials.ts +++ b/packages/cli/src/commands/import/credentials.ts @@ -13,9 +13,8 @@ import { BaseCommand } from '../BaseCommand'; import type { ICredentialsEncrypted } from 'n8n-workflow'; import { ApplicationError, jsonParse } from 'n8n-workflow'; import { UM_FIX_INSTRUCTION } from '@/constants'; -import { UserRepository } from '@db/repositories/user.repository'; import { ProjectRepository } from '@/databases/repositories/project.repository'; -import type { Project } from '@/databases/entities/Project'; +import { Project } from '@/databases/entities/Project'; import { User } from '@/databases/entities/User'; export class ImportCredentialsCommand extends BaseCommand { @@ -76,13 +75,13 @@ export class ImportCredentialsCommand extends BaseCommand { ); } - const project = await this.getProject(flags.userId, flags.projectId); - const credentials = await this.readCredentials(flags.input, flags.separate); await Db.getConnection().transaction(async (transactionManager) => { this.transactionManager = transactionManager; + const project = await this.getProject(flags.userId, flags.projectId); + const result = await this.checkRelations(credentials, flags.projectId, flags.userId); if (!result.success) { @@ -131,19 +130,6 @@ export class ImportCredentialsCommand extends BaseCommand { } } - private async getOwnerProject() { - const owner = await Container.get(UserRepository).findOneBy({ role: 'global:owner' }); - if (!owner) { - throw new ApplicationError(`Failed to find owner. ${UM_FIX_INSTRUCTION}`); - } - - const project = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( - owner.id, - ); - - return project; - } - private async checkRelations( credentials: ICredentialsEncrypted[], projectId?: string, @@ -264,13 +250,20 @@ export class ImportCredentialsCommand extends BaseCommand { private async getProject(userId?: string, projectId?: string) { if (projectId) { - return await Container.get(ProjectRepository).findOneByOrFail({ id: projectId }); + return await this.transactionManager.findOneByOrFail(Project, { id: projectId }); } - if (userId) { - return await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(userId); + if (!userId) { + const owner = await this.transactionManager.findOneBy(User, { role: 'global:owner' }); + if (!owner) { + throw new ApplicationError(`Failed to find owner. ${UM_FIX_INSTRUCTION}`); + } + userId = owner.id; } - return await this.getOwnerProject(); + return await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( + userId, + this.transactionManager, + ); } } diff --git a/packages/cli/src/databases/repositories/project.repository.ts b/packages/cli/src/databases/repositories/project.repository.ts index faae0bb9cf8e7..086dfbc7cf00b 100644 --- a/packages/cli/src/databases/repositories/project.repository.ts +++ b/packages/cli/src/databases/repositories/project.repository.ts @@ -17,8 +17,10 @@ export class ProjectRepository extends Repository { }); } - async getPersonalProjectForUserOrFail(userId: string) { - return await this.findOneOrFail({ + async getPersonalProjectForUserOrFail(userId: string, entityManager?: EntityManager) { + const em = entityManager ?? this.manager; + + return await em.findOneOrFail(Project, { where: { type: 'personal', projectRelations: { userId, role: 'project:personalOwner' } }, }); } From cf7bb3b1ad4dd683a7d527a0977a815481e1aa9c 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: Wed, 22 May 2024 13:38:08 +0200 Subject: [PATCH 8/9] more fixes --- .../credentials/credentials.service.ts | 1 + packages/cli/src/commands/import/workflow.ts | 23 ++++++------------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts index 6a7cfa208d802..20b7d5f949488 100644 --- a/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts +++ b/packages/cli/src/PublicApi/v1/handlers/credentials/credentials.service.ts @@ -69,6 +69,7 @@ export async function saveCredential( const personalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( user.id, + transactionManager, ); Object.assign(newSharedCredential, { diff --git a/packages/cli/src/commands/import/workflow.ts b/packages/cli/src/commands/import/workflow.ts index 7a6b7c38f2426..87bb590d6b09c 100644 --- a/packages/cli/src/commands/import/workflow.ts +++ b/packages/cli/src/commands/import/workflow.ts @@ -160,19 +160,6 @@ export class ImportWorkflowsCommand extends BaseCommand { this.logger.info(`Successfully imported ${total} ${total === 1 ? 'workflow.' : 'workflows.'}`); } - private async getOwnerProject() { - const owner = await Container.get(UserRepository).findOneBy({ role: 'global:owner' }); - if (!owner) { - throw new ApplicationError(`Failed to find owner. ${UM_FIX_INSTRUCTION}`); - } - - const project = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( - owner.id, - ); - - return project; - } - private async getWorkflowOwner(workflow: WorkflowEntity) { const sharing = await Container.get(SharedWorkflowRepository).findOne({ where: { workflowId: workflow.id, role: 'workflow:owner' }, @@ -234,10 +221,14 @@ export class ImportWorkflowsCommand extends BaseCommand { return await Container.get(ProjectRepository).findOneByOrFail({ id: projectId }); } - if (userId) { - return await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(userId); + if (!userId) { + const owner = await Container.get(UserRepository).findOneBy({ role: 'global:owner' }); + if (!owner) { + throw new ApplicationError(`Failed to find owner. ${UM_FIX_INSTRUCTION}`); + } + userId = owner.id; } - return await this.getOwnerProject(); + return await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(userId); } } From f57bb623836187998b86a4d0a8d1623cb347dc2a 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: Wed, 22 May 2024 14:30:30 +0200 Subject: [PATCH 9/9] revert sqlite-pooled pool-size --- .github/workflows/ci-postgres-mysql.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-postgres-mysql.yml b/.github/workflows/ci-postgres-mysql.yml index 6389da78e9c61..9cf864b7074e2 100644 --- a/.github/workflows/ci-postgres-mysql.yml +++ b/.github/workflows/ci-postgres-mysql.yml @@ -42,7 +42,7 @@ jobs: timeout-minutes: 20 env: DB_TYPE: sqlite - DB_SQLITE_POOL_SIZE: 1 + DB_SQLITE_POOL_SIZE: 4 steps: - uses: actions/checkout@v4.1.1 - run: corepack enable