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

insertByAll race condition? #64

Open
MichaelXavier opened this issue Jan 24, 2017 · 1 comment
Open

insertByAll race condition? #64

MichaelXavier opened this issue Jan 24, 2017 · 1 comment

Comments

@MichaelXavier
Copy link

I implemented some Haskell logic based on the docs of insertByAll saying if there was a constraint violation that it would return Left and then an arbitrary id matching the constraint. I assumed it was rescuing the db-specific error code for a constraint violation. Instead it looks like its selecting ids that match the constraint (no doubt necessary because the db vendor's error message won't have this info). I think the problem here is there is a race condition between the check and the insert:

  1. Thread 1 selects ids matching constraints, it gets none.
  2. Thread 2 selects ids matching constraints, it gets none.
  3. Thread 2 inserts and commits the transaction.
  4. Thread 1 tries to insert, but gets a unique constraint violation. The transaction rolls back and rather than returning a conflicting ID it throws a postgres (in my case) exception.

I'm not certain what the correct solution is here but this feels like it violates the spirit of the documentation. I'd expect not to ever receive a constraint exception fron this function.

In one option you could just insert blindly, rescue a uniqueness error (which means you can't fully get away with one RDBMS-agnostic insertByAll) and then issue your conflicting id query. Problem here is will abort the larger transaction and I'm not familiar enough with transaction semantics in groundhog to know if this is okay.

In another option, you create a subtransaction for the insert. You could still check, as the postgres IRC channel seems to believe the subtransaction is expensive enough that you'd want to optimally avoid it with the pre-insert check. You'd still have to catch the constraint violation.

There may be other solutions, such as this crazy upsert in postgresql.

@andrewthad
Copy link
Contributor

I just got bit by this as well. At the least, the docs should be improved to indicate that this function has a race condition.

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

No branches or pull requests

2 participants