fix for race condition when multiple processes try to add the same tag #499
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
👍 |
| bob.tag_list << "happier" | ||
| bob.save | ||
| }.should change(ActsAsTaggableOn::Tagging, :count).by(1) | ||
| describe "Duplicates" do |
There was a problem hiding this comment.
i think this should be a context
|
👍 |
- Raising the exception from save_tags is better than catching all exceptions. The client can retry the save on duplicate error. - add `extend self` to ActsAsTaggableOn::Utils so module functions can be called directly
…d on CI if there is an unintended failure.
…sql with retry was a good idea anyway. Now always raise ActsAsTaggableOn::DuplicateTagError when ActiveRecord::RecordNotUnique is thrown.
|
The multithreaded tests are causing deadlocks on Travis for MySQL. I don't have time to work on this right now. I will get back to this in a couple of days. Sorry for the delay. |
|
I set this PR for the version 3.2.0 . It passing all tests now. |
seuros
added a commit
that referenced
this pull request
Apr 1, 2014
…g-tags fix for race condition when multiple processes try to add the same tag
tekniklr
pushed a commit
to tekniklr/acts-as-taggable-on
that referenced
this pull request
Mar 19, 2021
…n-adding-tags fix for race condition when multiple processes try to add the same tag
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It is possible for a site using tags to have multiple web servers and multiple job processes. In this case it is also likely that more than one process could try to add the same tag at the same time. Now that there is a unique index on tag name, no duplicates are created, but a cryptic
ActiveRecord::RecordNotUniqueerror makes it up the client code where the taggable was saved. It is hard for the client to know that theActiveRecord::RecordNotUniquemust be caught and a simple re save will succeed. This PR is an attempt to make that process easier.While testing this I found that Postgres and MySQL behave differently when the unique index is violated. Postgres aborts the current transaction, so no more database operations can be executed. For Postgres this means we need to get an exception up to the client. I chose to add an exception to the library so clients can catch and retry their operation. For MySQL we can catch
ActiveRecord::RecordNotUniqueand request the 'duplicates' again.