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 stonk deadlock by adding missing select_for_update #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

interru
Copy link

@interru interru commented Jun 21, 2020

Rules

For my solution I set myself the following additional rules:

  • I have to use the functions which are marked as "Do not change".
  • I have to treat the functions as black boxes.
  • I have to assume that there are additional transactions I don't know about.
  • I only focus on the problem described in the README.

I set these rules because otherwise my preferred solution would be to use
bulk updates. But based on the context I doubt that this is what you want
to see here.

Solutions

Potential ways to solve the database deadlock:

  1. Remove the atomic.

    Every stonk gets updated in a seperate transaction.

    Problem: This only works if we assume that stonks won't get deleted
    and is a really dirty fix which could cause inconsistent states.

  2. Update in the same order

    Deadlocks only happen if two or multiple updates (locks) in transactions are
    interleaving and therefore blocking each other.
    If the updates are ordered and there is an overlap one transaction will
    secure the row level lock first and because of the ordering
    the rest of the data is unlocked. Resolving the problem in this case.

    Problem: This only works if we assume that every transaction does ordered
    updates.

  3. Select for update

    Tell the Database that we want to update the selected data.
    This will (on query execution) create a row level lock for every
    stonk we fetch. If there is already a lock held by another transaction
    for a row the query blocks until the lock is released.

I prefer the select for update solution in this case because it most precisely shows the
intent and doesn't rely on assumptions.

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