Merge branch '54117-transactional-rebase' into 'master'

Allow asynchronous rebase operations to be monitored

Closes #54117

See merge request gitlab-org/gitlab-ce!29940

4 files changed, 33 insertions, 3 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 7ee8e0ea8f8..2aa2508be16 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -201,7 +201,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   end
 
   def rebase
-    RebaseWorker.perform_async(@merge_request.id, current_user.id)
+    @merge_request.rebase_async(current_user.id)
 
     head :ok
   end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 8391d526d18..e96e26cc773 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -223,7 +223,13 @@ class MergeRequest < ApplicationRecord
   end
 
   def rebase_in_progress?
-    strong_memoize(:rebase_in_progress) do
+    (rebase_jid.present? && Gitlab::SidekiqStatus.running?(rebase_jid)) ||
+      gitaly_rebase_in_progress?
+  end
+
+  # TODO: remove the Gitaly lookup after v12.1, when rebase_jid will be reliable
+  def gitaly_rebase_in_progress?
+    strong_memoize(:gitaly_rebase_in_progress) do
       # The source project can be deleted
       next false unless source_project
 
@@ -389,6 +395,26 @@ class MergeRequest < ApplicationRecord
     update_column(:merge_jid, jid)
   end
 
+  # Set off a rebase asynchronously, atomically updating the `rebase_jid` of
+  # the MR so that the status of the operation can be tracked.
+  def rebase_async(user_id)
+    transaction do
+      lock!
+
+      raise ActiveRecord::StaleObjectError if !open? || rebase_in_progress?
+
+      # Although there is a race between setting rebase_jid here and clearing it
+      # in the RebaseWorker, it can't do any harm since we check both that the
+      # attribute is set *and* that the sidekiq job is still running. So a JID
+      # for a completed RebaseWorker is equivalent to a nil JID.
+      jid = Sidekiq::Worker.skipping_transaction_check do
+        RebaseWorker.perform_async(id, user_id)
+      end
+
+      update_column(:rebase_jid, jid)
+    end
+  end
+
   def merge_participants
     participants = [author]
 
diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb
index 4b9921c28ba..8d3b9b05819 100644
--- a/app/services/merge_requests/rebase_service.rb
+++ b/app/services/merge_requests/rebase_service.rb
@@ -15,7 +15,7 @@ module MergeRequests
     end
 
     def rebase
-      if merge_request.rebase_in_progress?
+      if merge_request.gitaly_rebase_in_progress?
         log_error('Rebase task canceled: Another rebase is already in progress', save_message_on_model: true)
         return false
       end
@@ -27,6 +27,8 @@ module MergeRequests
       log_error(REBASE_ERROR, save_message_on_model: true)
       log_error(e.message)
       false
+    ensure
+      merge_request.update_column(:rebase_jid, nil)
     end
   end
 end
diff --git a/app/workers/rebase_worker.rb b/app/workers/rebase_worker.rb
index a6baebc1443..8d06adcd993 100644
--- a/app/workers/rebase_worker.rb
+++ b/app/workers/rebase_worker.rb
@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+# The RebaseWorker must be wrapped in important concurrency code, so should only
+# be scheduled via MergeRequest#rebase_async
 class RebaseWorker
   include ApplicationWorker