diff options
| author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-10-17 18:00:02 +0000 |
|---|---|---|
| committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-10-17 18:00:02 +0000 |
| commit | 64c2bccdd90a62131c57a9c43deb34cab4e5bcbd (patch) | |
| tree | 376108b84ca2b02f28d0d341064aca0d9675ae1c | |
| parent | 5ad3a274b3404286bb32b695c8f2b7bdd21e4953 (diff) | |
| parent | ca3ce5c26c60de421e010491cf9166f2090c8cc8 (diff) | |
| download | gitlab-ce-64c2bccdd90a62131c57a9c43deb34cab4e5bcbd.tar.gz | |
Merge branch 'fix-issue-3067' into 'master'
Fix nonatomic database update potentially causing project star counts to go negative
The `counter_cache` decrement function is called when a project star is deleted,
but there was no guarantee multiple workers would not attempt to delete the
same item simultaneously. Use an atomic update to prevent the count from going negative.
Note: I was only able to duplicate the issue on GitLab.com, but I haven't been able to replicate it anywhere else. Presumably this bug happens when the `WHERE` clause takes some time to search for the right entry and multiple requests get sent simultaneously. Need to verify whether this fix truly solves the issue.
Closes #3067
See merge request !1628
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/models/user.rb | 15 |
2 files changed, 10 insertions, 6 deletions
diff --git a/CHANGELOG b/CHANGELOG index a1161a7a527..2c8df909441 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ v 8.2.0 (unreleased) - Highlight comment based on anchor in URL v 8.1.0 (unreleased) + - Fix nonatomic database update potentially causing project star counts to go negative (Stan Hu) - Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu) - Speed up load times of issue detail pages by roughly 1.5x - Add a system note and update relevant merge requests when a branch is deleted or re-added (Stan Hu) diff --git a/app/models/user.rb b/app/models/user.rb index 17ccb3b8788..3b346c55edb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -706,12 +706,15 @@ class User < ActiveRecord::Base end def toggle_star(project) - user_star_project = users_star_projects. - where(project: project, user: self).take - if user_star_project - user_star_project.destroy - else - UsersStarProject.create!(project: project, user: self) + UsersStarProject.transaction do + user_star_project = users_star_projects. + where(project: project, user: self).lock(true).first + + if user_star_project + user_star_project.destroy + else + UsersStarProject.create!(project: project, user: self) + end end end |