Add latest changes from gitlab-org/gitlab@master

19 files changed, 150 insertions, 5 deletions

diff --git a/Gemfile b/Gemfile
index eaea54cf8da..05c35df878f 100644
--- a/Gemfile
+++ b/Gemfile
@@ -469,3 +469,5 @@ gem 'gitlab-net-dns', '~> 0.9.1'
 
 # Countries list
 gem 'countries', '~> 3.0'
+
+gem 'retriable', '~> 3.1.2'
diff --git a/Gemfile.lock b/Gemfile.lock
index 902a8281101..fab1e5347dc 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1276,6 +1276,7 @@ DEPENDENCIES
   redis-rails (~> 5.0.2)
   request_store (~> 1.3)
   responders (~> 2.0)
+  retriable (~> 3.1.2)
   rouge (~> 3.11.0)
   rqrcode-rails3 (~> 0.1.7)
   rspec-parameterized
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue
index 339e154affc..57be97855e3 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_rebase.vue
@@ -65,9 +65,13 @@ export default {
           simplePoll(this.checkRebaseStatus);
         })
         .catch(error => {
-          this.rebasingError = error.merge_error;
           this.isMakingRequest = false;
-          Flash(__('Something went wrong. Please try again.'));
+
+          if (error.response && error.response.data && error.response.data.merge_error) {
+            this.rebasingError = error.response.data.merge_error;
+          } else {
+            Flash(__('Something went wrong. Please try again.'));
+          }
         });
     },
     checkRebaseStatus(continuePolling, stopPolling) {
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index e6032323c13..8d388151dbc 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -226,6 +226,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     @merge_request.rebase_async(current_user.id)
 
     head :ok
+  rescue MergeRequest::RebaseLockTimeout => e
+    render json: { merge_error: e.message }, status: :conflict
   end
 
   def discussions
diff --git a/app/helpers/environments_helper.rb b/app/helpers/environments_helper.rb
index c642a64ad61..f57d0fa19d4 100644
--- a/app/helpers/environments_helper.rb
+++ b/app/helpers/environments_helper.rb
@@ -34,6 +34,7 @@ module EnvironmentsHelper
       "project-path" => project_path(project),
       "tags-path" => project_tags_path(project),
       "has-metrics" => "#{environment.has_metrics?}",
+      "prometheus-status" => "#{environment.prometheus_status}",
       "external-dashboard-url" => project.metrics_setting_external_dashboard_url
     }
   end
diff --git a/app/models/environment.rb b/app/models/environment.rb
index af0c219d9a0..b426941d8af 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -188,6 +188,10 @@ class Environment < ApplicationRecord
     prometheus_adapter.query(:environment, self) if has_metrics?
   end
 
+  def prometheus_status
+    deployment_platform&.cluster&.application_prometheus&.status_name
+  end
+
   def additional_metrics(*args)
     return unless has_metrics?
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 6ef84c5f59b..b1c7e778743 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -220,6 +220,10 @@ class MergeRequest < ApplicationRecord
   alias_attribute :auto_merge_enabled, :merge_when_pipeline_succeeds
   alias_method :issuing_parent, :target_project
 
+  RebaseLockTimeout = Class.new(StandardError)
+
+  REBASE_LOCK_MESSAGE = _("Failed to enqueue the rebase operation, possibly due to a long-lived transaction. Try again later.")
+
   def self.reference_prefix
     '!'
   end
@@ -409,9 +413,7 @@ class MergeRequest < ApplicationRecord
   # 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!
-
+    with_rebase_lock do
       raise ActiveRecord::StaleObjectError if !open? || rebase_in_progress?
 
       # Although there is a race between setting rebase_jid here and clearing it
@@ -1468,6 +1470,30 @@ class MergeRequest < ApplicationRecord
 
   private
 
+  def with_rebase_lock
+    if Feature.enabled?(:merge_request_rebase_nowait_lock, default_enabled: true)
+      with_retried_nowait_lock { yield }
+    else
+      with_lock(true) { yield }
+    end
+  end
+
+  # If the merge request is idle in transaction or has a SELECT FOR
+  # UPDATE, we don't want to block indefinitely or this could cause a
+  # queue of SELECT FOR UPDATE calls. Instead, try to get the lock for
+  # 5 s before raising an error to the user.
+  def with_retried_nowait_lock
+    # Try at most 0.25 + (1.5 * .25) + (1.5^2 * .25) ... (1.5^5 * .25) = 5.2 s to get the lock
+    Retriable.retriable(on: ActiveRecord::LockWaitTimeout, tries: 6, base_interval: 0.25) do
+      with_lock('FOR UPDATE NOWAIT') do
+        yield
+      end
+    end
+  rescue ActiveRecord::LockWaitTimeout => e
+    Gitlab::Sentry.track_acceptable_exception(e)
+    raise RebaseLockTimeout, REBASE_LOCK_MESSAGE
+  end
+
   def source_project_variables
     Gitlab::Ci::Variables::Collection.new.tap do |variables|
       break variables unless source_project
diff --git a/changelogs/unreleased/sh-time-limit-merge-rebase-lock.yml b/changelogs/unreleased/sh-time-limit-merge-rebase-lock.yml
new file mode 100644
index 00000000000..c96db4d2ea9
--- /dev/null
+++ b/changelogs/unreleased/sh-time-limit-merge-rebase-lock.yml
@@ -0,0 +1,5 @@
+---
+title: Time limit the database lock when rebasing a merge request
+merge_request: 18481
+author:
+type: fixed
diff --git a/changelogs/unreleased/show-prometheus-is-updating.yml b/changelogs/unreleased/show-prometheus-is-updating.yml
new file mode 100644
index 00000000000..a62c0eb26ac
--- /dev/null
+++ b/changelogs/unreleased/show-prometheus-is-updating.yml
@@ -0,0 +1,5 @@
+---
+title: Expose prometheus status to monitor dashboard
+merge_request: 18289
+author:
+type: fixed
diff --git a/doc/user/group/epics/img/epic_view_roadmap_v12.3.png b/doc/user/group/epics/img/epic_view_roadmap_v12.3.png
index a17c56c618b..a17c56c618b 100755..100644
--- a/doc/user/group/epics/img/epic_view_roadmap_v12.3.png
+++ b/doc/user/group/epics/img/epic_view_roadmap_v12.3.png
diff --git a/doc/user/group/epics/img/epic_view_v12.3.png b/doc/user/group/epics/img/epic_view_v12.3.png
index 79758cf3d52..79758cf3d52 100755..100644
--- a/doc/user/group/epics/img/epic_view_v12.3.png
+++ b/doc/user/group/epics/img/epic_view_v12.3.png
diff --git a/doc/user/group/epics/img/epics_list_view_v12.3.png b/doc/user/group/epics/img/epics_list_view_v12.3.png
index c6817a503e7..c6817a503e7 100755..100644
--- a/doc/user/group/epics/img/epics_list_view_v12.3.png
+++ b/doc/user/group/epics/img/epics_list_view_v12.3.png
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 1436238c5cf..a18380586a7 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -455,6 +455,8 @@ module API
 
         status :accepted
         present rebase_in_progress: merge_request.rebase_in_progress?
+      rescue ::MergeRequest::RebaseLockTimeout => e
+        render_api_error!(e.message, 409)
       end
 
       desc 'List issues that will be closed on merge' do
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 6f66b51951d..d97a149805b 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -6846,6 +6846,9 @@ msgstr ""
 msgid "Failed to deploy to"
 msgstr ""
 
+msgid "Failed to enqueue the rebase operation, possibly due to a long-lived transaction. Try again later."
+msgstr ""
+
 msgid "Failed to get ref."
 msgstr ""
 
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 827b34b8850..74834e86467 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -1409,6 +1409,33 @@ describe Projects::MergeRequestsController do
       end
     end
 
+    context 'with SELECT FOR UPDATE lock' do
+      before do
+        stub_feature_flags(merge_request_rebase_nowait_lock: false)
+      end
+
+      it 'executes rebase' do
+        allow_any_instance_of(MergeRequest).to receive(:with_lock).with(true).and_call_original
+        expect(RebaseWorker).to receive(:perform_async)
+
+        post_rebase
+
+        expect(response.status).to eq(200)
+      end
+    end
+
+    context 'with NOWAIT lock' do
+      it 'returns a 409' do
+        allow_any_instance_of(MergeRequest).to receive(:with_lock).with('FOR UPDATE NOWAIT').and_raise(ActiveRecord::LockWaitTimeout)
+        expect(RebaseWorker).not_to receive(:perform_async)
+
+        post_rebase
+
+        expect(response.status).to eq(409)
+        expect(json_response['merge_error']).to eq(MergeRequest::REBASE_LOCK_MESSAGE)
+      end
+    end
+
     context 'with a forked project' do
       let(:forked_project) { fork_project(project, fork_owner, repository: true) }
       let(:fork_owner) { create(:user) }
diff --git a/spec/helpers/environments_helper_spec.rb b/spec/helpers/environments_helper_spec.rb
index 2b8bf9319fc..a50c8e9bf8e 100644
--- a/spec/helpers/environments_helper_spec.rb
+++ b/spec/helpers/environments_helper_spec.rb
@@ -32,6 +32,7 @@ describe EnvironmentsHelper do
         'project-path' => project_path(project),
         'tags-path' => project_tags_path(project),
         'has-metrics' => "#{environment.has_metrics?}",
+        'prometheus-status' => "#{environment.prometheus_status}",
         'external-dashboard-url' => nil
       )
     end
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 786f3b832c4..85a8f35393b 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -727,6 +727,51 @@ describe Environment, :use_clean_rails_memory_store_caching do
     end
   end
 
+  describe '#prometheus_status' do
+    context 'when a cluster is present' do
+      context 'when a deployment platform is present' do
+        let(:cluster) { create(:cluster, :provided_by_user, :project) }
+        let(:environment) { create(:environment, project: cluster.project) }
+
+        subject { environment.prometheus_status }
+
+        context 'when the prometheus application status is :updating' do
+          let!(:prometheus) { create(:clusters_applications_prometheus, :updating, cluster: cluster) }
+
+          it { is_expected.to eq(:updating) }
+        end
+
+        context 'when the prometheus application state is :updated' do
+          let!(:prometheus) { create(:clusters_applications_prometheus, :updated, cluster: cluster) }
+
+          it { is_expected.to eq(:updated) }
+        end
+
+        context 'when the prometheus application is not installed' do
+          it { is_expected.to be_nil }
+        end
+      end
+
+      context 'when a deployment platform is not present' do
+        let(:cluster) { create(:cluster, :project) }
+        let(:environment) { create(:environment, project: cluster.project) }
+
+        subject { environment.prometheus_status }
+
+        it { is_expected.to be_nil }
+      end
+    end
+
+    context 'when a cluster is not present' do
+      let(:project) { create(:project, :stubbed_repository) }
+      let(:environment) { create(:environment, project: project) }
+
+      subject { environment.prometheus_status }
+
+      it { is_expected.to be_nil }
+    end
+  end
+
   describe '#additional_metrics' do
     let(:project) { create(:prometheus_project) }
     let(:metric_params) { [] }
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index b8d09f03fe6..e2a0acf85f6 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -2138,6 +2138,13 @@ describe MergeRequest do
 
       expect { execute }.to raise_error(ActiveRecord::StaleObjectError)
     end
+
+    it "raises ActiveRecord::LockWaitTimeout after 6 tries" do
+      expect(merge_request).to receive(:with_lock).exactly(6).times.and_raise(ActiveRecord::LockWaitTimeout)
+      expect(RebaseWorker).not_to receive(:perform_async)
+
+      expect { execute }.to raise_error(MergeRequest::RebaseLockTimeout)
+    end
   end
 
   describe '#mergeable?' do
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 8179da2f97c..e9bf8e7adc7 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -2120,6 +2120,16 @@ describe API::MergeRequests do
 
       expect(response).to have_gitlab_http_status(409)
     end
+
+    it "returns 409 if rebase can't lock the row" do
+      allow_any_instance_of(MergeRequest).to receive(:with_lock).and_raise(ActiveRecord::LockWaitTimeout)
+      expect(RebaseWorker).not_to receive(:perform_async)
+
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/rebase", user)
+
+      expect(response).to have_gitlab_http_status(409)
+      expect(json_response['message']).to eq(MergeRequest::REBASE_LOCK_MESSAGE)
+    end
   end
 
   describe 'Time tracking' do