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

How to deal with race conditions caused by concurrency #693

Closed
kshahkshah opened this issue Oct 27, 2015 · 7 comments · Fixed by #809
Closed

How to deal with race conditions caused by concurrency #693

kshahkshah opened this issue Oct 27, 2015 · 7 comments · Fixed by #809

Comments

@kshahkshah
Copy link

2015-10-27T15:33:42.642Z 3123 TID-oxfjnp9pw WARN: ActsAsTaggableOn::DuplicateTagError: 'orange' has already been taken

As part of my project I'm working on I use Sidekiq to run a tagger on 25,000 AR-backed models in postgres. With the default concurrency of 25 workers I instantly run into the above problem every single time.

I assume the responsible code is L74 of https://github.com/mbleigh/acts-as-taggable-on/blob/fd28222629ad59f5a3958424e66e5b9f81a5be18/lib/acts_as_taggable_on/tag.rb

which is existing_tag || create(name: tag_name)

By the time existing_tag returns false another worker has created the tag and then create fails.

There are a number of solutions to the race condition and I'm happy to implement. But does anyone have an opinion here? Would a patch be accepted?

Something perhaps like insert/select/where not exists?

@kshahkshah
Copy link
Author

Also, if the DuplicateTagError gets raised perhaps retry the fetch and return the good tag?

@juliankrispel
Copy link

We seem to have a similar problem. The error message we get is 'Tag has already been taken'

@jdelStrother
Copy link
Contributor

Also, if the DuplicateTagError gets raised perhaps retry the fetch and return the good tag?

I think the problem there might be that since you're inside a transaction at that point (eg due to ActiveRecord::Base#save), you won't be able to see the results of the other transaction that created the tag, right?

@adam-e-trepanier
Copy link

This bit me today. Would it be possible to lock the table prior to reading existing tags and creating?

@adam-e-trepanier
Copy link

Here is a 'hacked' version that allows for retry. Admittedly not a perfect solution, but it works for the time being. https://github.com/adam-e-trepanier/acts-as-taggable-on/tree/double-check-tag-doesnt-exist

@jdelStrother
Copy link
Contributor

jdelStrother commented Oct 5, 2016

@adam-e-trepanier hmm, I'm still having problems with that approach - eg with Post.find(1).update_attributes(tag_list: 'x'), retry_find_or_create_all_with_link_by_name is going to occur within the transaction that ActiveRecord wraps any save calls with, so you'll still get a DuplicateTagError on the retry.

@adam-e-trepanier
Copy link

@jdelStrother Yes, you are correct. please disregard. I am working on something to try nested transactions. Ill let you know if that works.

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 a pull request may close this issue.

4 participants