Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: PG::TRDeadlockDetected (MAYBE-RAILS-CE) #1621

Closed
wants to merge 1 commit into from

Conversation

revise-dev[bot]
Copy link

@revise-dev revise-dev bot commented Jan 15, 2025

The deadlock issue occurs during concurrent inserts into the transfers table due to the unique index on inflow/outflow transaction IDs. When multiple processes try to create transfers simultaneously, they can attempt to acquire locks on the same transaction IDs but in different orders, leading to a circular wait condition.

Key changes made to the auto_match_for_account method:

  1. Added consistent transaction ID ordering:

    • Before: The inflow/outflow assignment was based solely on amount signs
    • After: First sorts transaction IDs numerically, then adjusts based on amount direction
    • This ensures all processes attempt to acquire locks in the same order
  2. Modified the transaction ID assignment logic:

    • Uses array sorting to guarantee consistent ordering ([t1_id, t2_id].sort)
    • Then applies amount-based direction adjustment
    • This prevents the scenario where Process A tries to lock (1,2) while Process B tries to lock (2,1)
  3. Maintains existing business logic:

    • Still preserves the amount-based directionality (positive amounts = outflow)
    • Keeps the same matching criteria (amount, currency, date range)
    • Retains duplicate prevention logic

Added comprehensive test coverage:

  • Tests concurrent execution with multiple threads
  • Verifies correct transfer creation
  • Ensures no deadlocks occur
  • Validates proper transaction ID assignment

The original intent of the code was to match transfers based on opposing amounts within a date range, but it didn't account for concurrent execution patterns. The updated version maintains this functionality while adding thread-safety through consistent lock ordering.

Tip

You can make revisions or ask questions of Revise.dev by using /revise in any comment or review!

  • /revise Add a comment above the method to explain why we're making this change.
  • /revise Why did you choose to make this change specifically?

@zachgoll zachgoll closed this Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant