diff options
author | Michael Kozono <mkozono@gmail.com> | 2017-05-06 00:31:33 +0300 |
---|---|---|
committer | Michael Kozono <mkozono@gmail.com> | 2017-05-06 00:31:33 +0300 |
commit | b0ee22609a89572d6e3f98eebccf9fb2335dd939 (patch) | |
tree | 3fb75c0f2eea7ff932a127bb416c130db3d7ce20 /app | |
parent | e1c245af51e294c84552cff8021342e7ae493b8a (diff) |
Reduce risk of deadlocks
We’ve seen a deadlock in CI here https://gitlab.com/mkozono/gitlab-ce/builds/15644492#down-build-trace. This commit should not fix that particular failure, but perhaps it will avoid others.
* Don’t call delete_conflicting_redirects after update if the path wasn’t changed
* Rename descendants without using recursion again, so we can run delete_conflicting_redirects exactly once.
Diffstat (limited to 'app')
-rw-r--r-- | app/models/route.rb | 24 |
1 files changed, 17 insertions, 7 deletions
diff --git a/app/models/route.rb b/app/models/route.rb index b34cce9077a..12a7fa3d01b 100644 --- a/app/models/route.rb +++ b/app/models/route.rb @@ -8,19 +8,19 @@ class Route < ActiveRecord::Base presence: true, uniqueness: { case_sensitive: false } - after_save :delete_conflicting_redirects + after_create :delete_conflicting_redirects + after_update :delete_conflicting_redirects, if: :path_changed? after_update :create_redirect_for_old_path - after_update :rename_direct_descendant_routes + after_update :rename_descendants scope :inside_path, -> (path) { where('routes.path LIKE ?', "#{sanitize_sql_like(path)}/%") } - scope :direct_descendant_routes, -> (path) { where('routes.path LIKE ? AND routes.path NOT LIKE ?', "#{sanitize_sql_like(path)}/%", "#{sanitize_sql_like(path)}/%/%") } - def rename_direct_descendant_routes + def rename_descendants return unless path_changed? || name_changed? - direct_descendant_routes = self.class.direct_descendant_routes(path_was) + descendant_routes = self.class.inside_path(path_was) - direct_descendant_routes.each do |route| + descendant_routes.each do |route| attributes = {} if path_changed? && route.path.present? @@ -31,7 +31,17 @@ class Route < ActiveRecord::Base attributes[:name] = route.name.sub(name_was, name) end - route.update(attributes) unless attributes.empty? + if attributes.present? + old_path = route.path + + # Callbacks must be run manually + route.update_columns(attributes) + + # We are not calling route.delete_conflicting_redirects here, in hopes + # of avoiding deadlocks. The parent (self, in this method) already + # called it, which deletes conflicts for all descendants. + route.create_redirect(old_path) if attributes[:path] + end end end |