Skip to content

fix(core): Retry role mapping order races#29494

Open
haimingZZ wants to merge 1 commit into
n8n-io:masterfrom
haimingZZ:fix/role-mapping-create-retry-haimingzz
Open

fix(core): Retry role mapping order races#29494
haimingZZ wants to merge 1 commit into
n8n-io:masterfrom
haimingZZ:fix/role-mapping-create-retry-haimingzz

Conversation

@haimingZZ
Copy link
Copy Markdown
Contributor

Summary

  • retry role mapping rule creation when a concurrent create picks the same temporary (type, order) slot
  • re-read the current ordering before retrying so the second create can append/splice after the rule that won the race
  • add a backend concurrency-style regression test that starts two appends together and verifies both complete with contiguous ordering

Review notes

This builds on #28722. That change avoids ordinary requested-order collisions and serializes the UI, but two backend/API creates can still read the same snapshot and choose the same temporary order for the initial insert. This keeps the existing two-phase applyOrder flow and only retries unique insert conflicts after re-reading.

Validation

  • git diff --check
  • pnpm --filter n8n test:unit -- role-mapping-rule.service.ee.test.ts (not run: this worktree has no node_modules, so jest is not installed)

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Architecture diagram
sequenceDiagram
    participant API as API/Provisioning Logic
    participant Service as RoleMappingRuleService
    participant Repo as RoleMappingRuleRepository
    participant DB as Database (Postgres/SQLite)

    Note over API,DB: Role Mapping Rule Creation Flow

    API->>Service: create(dto)
    Service->>Repo: find(existing rules for type)
    Repo-->>Service: rules[]
    
    Service->>Service: Calculate tempOrder (max + 1)
    
    rect rgb(240, 240, 240)
    Note over Service,DB: NEW: Retry Logic for Concurrent Inserts
    loop Until Success or Max Retries
        Service->>Repo: save(new rule with tempOrder)
        
        alt Success Path
            Repo->>DB: INSERT rule
            DB-->>Repo: OK
            Repo-->>Service: saved entity
        else CHANGED: Conflict (Race Condition)
            DB-->>Repo: Error: Unique Constraint (type, order)
            Repo-->>Service: throws ConflictError
            Service->>Repo: NEW: re-read current rules
            Repo-->>Service: latest rules[]
            Service->>Service: NEW: Recalculate tempOrder
        end
    end
    end

    Note over Service,DB: Two-Phase Ordering (applyOrder)
    
    Service->>DB: BEGIN TRANSACTION
    
    rect rgb(230, 245, 255)
    Note over Service,DB: Phase 1: Vacate slots (Move to high offset)
    loop for each ruleId in reorderedIds
        Service->>DB: UPDATE order = offset + i
    end
    end

    rect rgb(230, 255, 230)
    Note over Service,DB: Phase 2: Normalize (Sequence from 0)
    loop for each ruleId in reorderedIds
        Service->>DB: UPDATE order = i
    end
    end

    Service->>DB: COMMIT
    
    Service-->>API: RoleMappingRuleResponse
Loading

@n8n-assistant n8n-assistant Bot added community Authored by a community member core Enhancement outside /nodes-base and /editor-ui in linear DEPRECATED labels Apr 29, 2026
@n8n-assistant
Copy link
Copy Markdown
Contributor

n8n-assistant Bot commented Apr 29, 2026

Hey @haimingZZ,

Thank you for your contribution. We appreciate the time and effort you’ve taken to submit this pull request.

Before we can proceed, please ensure the following:
• Tests are included for any new functionality, logic changes or bug fixes.
• The PR aligns with our contribution guidelines.

Regarding new nodes:
We no longer accept new nodes directly into the core codebase. Instead, we encourage contributors to follow our Community Node Submission Guide to publish nodes independently.

If your node integrates with an AI service that you own or represent, please email nodes@n8n.io and we will be happy to discuss the best approach.

About review timelines:
This PR has been added to our internal tracker as "GHC-8063". While we plan to review it, we are currently unable to provide an exact timeframe. Our goal is to begin reviews within a month, but this may change depending on team priorities. We will reach out when the review begins.

Thank you again for contributing to n8n.

@haimingZZ haimingZZ force-pushed the fix/role-mapping-create-retry-haimingzz branch from f1b6921 to fcec9cd Compare April 29, 2026 12:36
@cla-bot cla-bot Bot added the cla-signed label May 11, 2026
@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented May 11, 2026

The cla-bot has been summoned, and re-checked this pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed community Authored by a community member core Enhancement outside /nodes-base and /editor-ui in linear DEPRECATED

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant