Merge branch 'sidekiq-transaction' into 'master'

Forbid Sidekiq scheduling in transactions

Closes #27233

See merge request !9376

6 files changed, 24 insertions, 14 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 3bcc8be5cae..effee3f48b1 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -47,8 +47,8 @@ module Ci
     before_destroy { unscoped_project }
 
     after_create :execute_hooks
-    after_save :update_project_statistics, if: :artifacts_size_changed?
-    after_destroy :update_project_statistics
+    after_commit :update_project_statistics_after_save, on: [:create, :update]
+    after_commit :update_project_statistics, on: :destroy
 
     class << self
       # This is needed for url_for to work,
@@ -512,5 +512,11 @@ module Ci
 
       ProjectCacheWorker.perform_async(project_id, [], [:build_artifacts_size])
     end
+
+    def update_project_statistics_after_save
+      if previous_changes.include?('artifacts_size')
+        update_project_statistics
+      end
+    end
   end
 end
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index fe63728ea23..ce507f7774b 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -18,7 +18,7 @@ class CommitStatus < ActiveRecord::Base
   validates :name, presence: true
 
   alias_attribute :author, :user
-  
+
   scope :failed_but_allowed, -> do
     where(allow_failure: true, status: [:failed, :canceled])
   end
@@ -83,14 +83,15 @@ class CommitStatus < ActiveRecord::Base
       next if transition.loopback?
 
       commit_status.run_after_commit do
-        pipeline.try do |pipeline|
+        if pipeline
           if complete? || manual?
             PipelineProcessWorker.perform_async(pipeline.id)
           else
             PipelineUpdateWorker.perform_async(pipeline.id)
           end
-          ExpireJobCacheWorker.perform_async(commit_status.id)
         end
+
+        ExpireJobCacheWorker.perform_async(commit_status.id)
       end
     end
 
diff --git a/app/models/key.rb b/app/models/key.rb
index b7956052c3f..cb8f10f6d55 100644
--- a/app/models/key.rb
+++ b/app/models/key.rb
@@ -1,7 +1,6 @@
 require 'digest/md5'
 
 class Key < ActiveRecord::Base
-  include AfterCommitQueue
   include Sortable
 
   LAST_USED_AT_REFRESH_TIME = 1.day.to_i
@@ -25,10 +24,10 @@ class Key < ActiveRecord::Base
 
   delegate :name, :email, to: :user, prefix: true
 
-  after_create :add_to_shell
-  after_create :notify_user
+  after_commit :add_to_shell, on: :create
+  after_commit :notify_user, on: :create
   after_create :post_create_hook
-  after_destroy :remove_from_shell
+  after_commit :remove_from_shell, on: :destroy
   after_destroy :post_destroy_hook
 
   def key=(value)
@@ -93,6 +92,6 @@ class Key < ActiveRecord::Base
   end
 
   def notify_user
-    run_after_commit { NotificationService.new.new_key(self) }
+    NotificationService.new.new_key(self)
   end
 end
diff --git a/app/models/lfs_objects_project.rb b/app/models/lfs_objects_project.rb
index 007eed5600a..b0625c52b62 100644
--- a/app/models/lfs_objects_project.rb
+++ b/app/models/lfs_objects_project.rb
@@ -6,8 +6,7 @@ class LfsObjectsProject < ActiveRecord::Base
   validates :lfs_object_id, uniqueness: { scope: [:project_id], message: "already exists in project" }
   validates :project_id, presence: true
 
-  after_create :update_project_statistics
-  after_destroy :update_project_statistics
+  after_commit :update_project_statistics, on: [:create, :destroy]
 
   private
 
diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index aebee06d560..b48d73dcae7 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -6,6 +6,7 @@ class Namespace < ActiveRecord::Base
   include Gitlab::ShellAdapter
   include Gitlab::CurrentSettings
   include Routable
+  include AfterCommitQueue
 
   # Prevent users from creating unreasonably deep level of nesting.
   # The number 20 was taken based on maximum nesting level of
@@ -242,7 +243,9 @@ class Namespace < ActiveRecord::Base
 
         # Remove namespace directroy async with delay so
         # GitLab has time to remove all projects first
-        GitlabShellWorker.perform_in(5.minutes, :rm_namespace, repository_storage_path, new_path)
+        run_after_commit do
+          GitlabShellWorker.perform_in(5.minutes, :rm_namespace, repository_storage_path, new_path)
+        end
       end
     end
 
diff --git a/app/models/project.rb b/app/models/project.rb
index 446329557d5..f16d1cab6c4 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -471,7 +471,9 @@ class Project < ActiveRecord::Base
   end
 
   def reset_cache_and_import_attrs
-    ProjectCacheWorker.perform_async(self.id)
+    run_after_commit do
+      ProjectCacheWorker.perform_async(self.id)
+    end
 
     self.import_data&.destroy
   end