fix(core): Detect DB connection aquisition deadlocks (no-changelog) (#9485)

Co-authored-by: Danny Martini <danny@n8n.io>
This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™
2024-05-22 14:53:23 +02:00
committed by GitHub
parent 2fa46b6faa
commit 3094f1b886
10 changed files with 53 additions and 66 deletions

View File

@@ -102,6 +102,7 @@ jobs:
timeout-minutes: 20 timeout-minutes: 20
env: env:
DB_POSTGRESDB_PASSWORD: password DB_POSTGRESDB_PASSWORD: password
DB_POSTGRESDB_POOL_SIZE: 1 # Detect connection pooling deadlocks
steps: steps:
- uses: actions/checkout@v4.1.1 - uses: actions/checkout@v4.1.1
- run: corepack enable - run: corepack enable

View File

@@ -69,6 +69,7 @@ export async function saveCredential(
const personalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( const personalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(
user.id, user.id,
transactionManager,
); );
Object.assign(newSharedCredential, { Object.assign(newSharedCredential, {

View File

@@ -7,7 +7,6 @@ import { SharedWorkflow, type WorkflowSharingRole } from '@db/entities/SharedWor
import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { WorkflowRepository } from '@db/repositories/workflow.repository';
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
import type { Project } from '@/databases/entities/Project'; import type { Project } from '@/databases/entities/Project';
import { WorkflowTagMappingRepository } from '@db/repositories/workflowTagMapping.repository';
import { TagRepository } from '@db/repositories/tag.repository'; import { TagRepository } from '@db/repositories/tag.repository';
import { License } from '@/License'; import { License } from '@/License';
import { WorkflowSharingService } from '@/workflows/workflowSharing.service'; 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<any> { export async function updateTags(workflowId: string, newTags: string[]): Promise<any> {
await Db.transaction(async (transactionManager) => { await Db.transaction(async (transactionManager) => {
const oldTags = await Container.get(WorkflowTagMappingRepository).findBy({ const oldTags = await transactionManager.findBy(WorkflowTagMapping, { workflowId });
workflowId,
});
if (oldTags.length > 0) { if (oldTags.length > 0) {
await transactionManager.delete(WorkflowTagMapping, oldTags); await transactionManager.delete(WorkflowTagMapping, oldTags);
} }

View File

@@ -3,7 +3,7 @@ import {
ErrorReporterProxy as ErrorReporter, ErrorReporterProxy as ErrorReporter,
WorkflowOperationError, WorkflowOperationError,
} from 'n8n-workflow'; } from 'n8n-workflow';
import { Container, Service } from 'typedi'; import { Service } from 'typedi';
import type { ExecutionStopResult, IWorkflowExecutionDataProcess } from '@/Interfaces'; import type { ExecutionStopResult, IWorkflowExecutionDataProcess } from '@/Interfaces';
import { WorkflowRunner } from '@/WorkflowRunner'; import { WorkflowRunner } from '@/WorkflowRunner';
import { ExecutionRepository } from '@db/repositories/execution.repository'; import { ExecutionRepository } from '@db/repositories/execution.repository';
@@ -137,10 +137,7 @@ export class WaitTracker {
fullExecutionData.waitTill = null; fullExecutionData.waitTill = null;
fullExecutionData.status = 'canceled'; fullExecutionData.status = 'canceled';
await Container.get(ExecutionRepository).updateExistingExecution( await this.executionRepository.updateExistingExecution(executionId, fullExecutionData);
executionId,
fullExecutionData,
);
return { return {
mode: fullExecutionData.mode, mode: fullExecutionData.mode,

View File

@@ -13,9 +13,9 @@ import { BaseCommand } from '../BaseCommand';
import type { ICredentialsEncrypted } from 'n8n-workflow'; import type { ICredentialsEncrypted } from 'n8n-workflow';
import { ApplicationError, jsonParse } from 'n8n-workflow'; import { ApplicationError, jsonParse } from 'n8n-workflow';
import { UM_FIX_INSTRUCTION } from '@/constants'; import { UM_FIX_INSTRUCTION } from '@/constants';
import { UserRepository } from '@db/repositories/user.repository';
import { ProjectRepository } from '@/databases/repositories/project.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 { export class ImportCredentialsCommand extends BaseCommand {
static description = 'Import credentials'; static description = 'Import credentials';
@@ -75,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); const credentials = await this.readCredentials(flags.input, flags.separate);
await Db.getConnection().transaction(async (transactionManager) => { await Db.getConnection().transaction(async (transactionManager) => {
this.transactionManager = transactionManager; this.transactionManager = transactionManager;
const project = await this.getProject(flags.userId, flags.projectId);
const result = await this.checkRelations(credentials, flags.projectId, flags.userId); const result = await this.checkRelations(credentials, flags.projectId, flags.userId);
if (!result.success) { if (!result.success) {
@@ -130,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( private async checkRelations(
credentials: ICredentialsEncrypted[], credentials: ICredentialsEncrypted[],
projectId?: string, projectId?: string,
@@ -244,7 +231,7 @@ export class ImportCredentialsCommand extends BaseCommand {
}); });
if (sharedCredential && sharedCredential.project.type === 'personal') { if (sharedCredential && sharedCredential.project.type === 'personal') {
const user = await Container.get(UserRepository).findOneByOrFail({ const user = await this.transactionManager.findOneByOrFail(User, {
projectRelations: { projectRelations: {
role: 'project:personalOwner', role: 'project:personalOwner',
projectId: sharedCredential.projectId, projectId: sharedCredential.projectId,
@@ -263,13 +250,20 @@ export class ImportCredentialsCommand extends BaseCommand {
private async getProject(userId?: string, projectId?: string) { private async getProject(userId?: string, projectId?: string) {
if (projectId) { if (projectId) {
return await Container.get(ProjectRepository).findOneByOrFail({ id: projectId }); return await this.transactionManager.findOneByOrFail(Project, { id: projectId });
} }
if (userId) { if (!userId) {
return await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(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,
);
} }
} }

View File

@@ -160,19 +160,6 @@ export class ImportWorkflowsCommand extends BaseCommand {
this.logger.info(`Successfully imported ${total} ${total === 1 ? 'workflow.' : 'workflows.'}`); 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) { private async getWorkflowOwner(workflow: WorkflowEntity) {
const sharing = await Container.get(SharedWorkflowRepository).findOne({ const sharing = await Container.get(SharedWorkflowRepository).findOne({
where: { workflowId: workflow.id, role: 'workflow:owner' }, where: { workflowId: workflow.id, role: 'workflow:owner' },
@@ -234,10 +221,14 @@ export class ImportWorkflowsCommand extends BaseCommand {
return await Container.get(ProjectRepository).findOneByOrFail({ id: projectId }); return await Container.get(ProjectRepository).findOneByOrFail({ id: projectId });
} }
if (userId) { if (!userId) {
return await Container.get(ProjectRepository).getPersonalProjectForUserOrFail(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);
} }
} }

View File

@@ -262,7 +262,10 @@ export class CredentialsService {
const project = const project =
projectId === undefined projectId === undefined
? await this.projectRepository.getPersonalProjectForUserOrFail(user.id) ? await this.projectRepository.getPersonalProjectForUserOrFail(
user.id,
transactionManager,
)
: await this.projectService.getProjectWithScope( : await this.projectService.getProjectWithScope(
user, user,
projectId, projectId,

View File

@@ -17,8 +17,10 @@ export class ProjectRepository extends Repository<Project> {
}); });
} }
async getPersonalProjectForUserOrFail(userId: string) { async getPersonalProjectForUserOrFail(userId: string, entityManager?: EntityManager) {
return await this.findOneOrFail({ const em = entityManager ?? this.manager;
return await em.findOneOrFail(Project, {
where: { type: 'personal', projectRelations: { userId, role: 'project:personalOwner' } }, where: { type: 'personal', projectRelations: { userId, role: 'project:personalOwner' } },
}); });
} }

View File

@@ -1,12 +1,12 @@
import { Container } from 'typedi';
import type { EntitySubscriberInterface, UpdateEvent } from '@n8n/typeorm'; import type { EntitySubscriberInterface, UpdateEvent } from '@n8n/typeorm';
import { EventSubscriber } 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 { ApplicationError, ErrorReporterProxy } from 'n8n-workflow';
import { Logger } from '@/Logger'; import { Logger } from '@/Logger';
import { UserRepository } from '../repositories/user.repository';
import { Project } from '../entities/Project'; import { Project } from '../entities/Project';
import { User } from '../entities/User';
import { UserRepository } from '../repositories/user.repository';
@EventSubscriber() @EventSubscriber()
export class UserSubscriber implements EntitySubscriberInterface<User> { export class UserSubscriber implements EntitySubscriberInterface<User> {
@@ -27,14 +27,17 @@ export class UserSubscriber implements EntitySubscriberInterface<User> {
fields.includes('email') fields.includes('email')
) { ) {
const oldUser = event.databaseEntity; const oldUser = event.databaseEntity;
const name = const userEntity =
newUserData instanceof User newUserData instanceof User
? newUserData.createPersonalProjectName() ? newUserData
: Container.get(UserRepository).create(newUserData).createPersonalProjectName(); : Container.get(UserRepository).create(newUserData);
const project = await Container.get(ProjectRepository).getPersonalProjectForUser( const projectName = userEntity.createPersonalProjectName();
oldUser.id,
); const project = await event.manager.findOneBy(Project, {
type: 'personal',
projectRelations: { userId: oldUser.id },
});
if (!project) { if (!project) {
// Since this is benign we're not throwing the exception. We don't // Since this is benign we're not throwing the exception. We don't
@@ -47,7 +50,7 @@ export class UserSubscriber implements EntitySubscriberInterface<User> {
return; return;
} }
project.name = name; project.name = projectName;
await event.manager.save(Project, project); await event.manager.save(Project, project);
} }

View File

@@ -1,4 +1,4 @@
import Container, { Service } from 'typedi'; import { Service } from 'typedi';
import { v4 as uuid } from 'uuid'; import { v4 as uuid } from 'uuid';
import { type INode, type INodeCredentialsDetails } from 'n8n-workflow'; 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 { TagRepository } from '@db/repositories/tag.repository';
import { SharedWorkflow } from '@db/entities/SharedWorkflow'; import { SharedWorkflow } from '@db/entities/SharedWorkflow';
import { replaceInvalidCredentials } from '@/WorkflowHelpers'; import { replaceInvalidCredentials } from '@/WorkflowHelpers';
import { Project } from '@db/entities/Project';
import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { WorkflowEntity } from '@db/entities/WorkflowEntity';
import { WorkflowTagMapping } from '@db/entities/WorkflowTagMapping'; import { WorkflowTagMapping } from '@db/entities/WorkflowTagMapping';
import type { TagEntity } from '@db/entities/TagEntity'; import type { TagEntity } from '@db/entities/TagEntity';
import type { ICredentialsDb } from '@/Interfaces'; import type { ICredentialsDb } from '@/Interfaces';
import { ProjectRepository } from '@/databases/repositories/project.repository';
@Service() @Service()
export class ImportService { export class ImportService {
@@ -59,9 +59,7 @@ export class ImportService {
const upsertResult = await tx.upsert(WorkflowEntity, workflow, ['id']); const upsertResult = await tx.upsert(WorkflowEntity, workflow, ['id']);
const workflowId = upsertResult.identifiers.at(0)?.id as string; const workflowId = upsertResult.identifiers.at(0)?.id as string;
const personalProject = await Container.get(ProjectRepository).findOneByOrFail({ const personalProject = await tx.findOneByOrFail(Project, { id: projectId });
id: projectId,
});
// Create relationship if the workflow was inserted instead of updated. // Create relationship if the workflow was inserted instead of updated.
if (!exists) { if (!exists) {