Merge branch 'remove-ci_new_query_for_pending_stuck_jobs-339322' into 'master'

Remove ci_new_query_for_pending_stuck_jobs feature flag

See merge request gitlab-org/gitlab!75420

app/models/commit_status.rb

@@ -59,9 +59,6 @@ class CommitStatus < Ci::ApplicationRecord
   scope :with_pipeline, -> { joins(:pipeline) }
   scope :updated_at_before, ->(date) { where('ci_builds.updated_at < ?', date) }
   scope :created_at_before, ->(date) { where('ci_builds.created_at < ?', date) }
-  scope :updated_before, ->(lookback:, timeout:) {
-    where('(ci_builds.created_at BETWEEN ? AND ?) AND (ci_builds.updated_at BETWEEN ? AND ?)', lookback, timeout, lookback, timeout)
-  }
   scope :scheduled_at_before, ->(date) {
     where('ci_builds.scheduled_at IS NOT NULL AND ci_builds.scheduled_at < ?', date)
   }

app/services/ci/stuck_builds/drop_pending_service.rb

@@ -7,7 +7,6 @@ module Ci
 
       BUILD_PENDING_OUTDATED_TIMEOUT = 1.day
       BUILD_PENDING_STUCK_TIMEOUT = 1.hour
-      BUILD_LOOKBACK = 5.days
 
       def execute
         Gitlab::AppLogger.info "#{self.class}: Cleaning pending timed-out builds"
@@ -30,11 +29,11 @@ module Ci
       # because we want to force the query planner to use the
       # `ci_builds_gitlab_monitor_metrics` index all the time.
       def pending_builds(timeout)
-        if Feature.enabled?(:ci_new_query_for_pending_stuck_jobs)
-          Ci::Build.pending.created_at_before(timeout).updated_at_before(timeout).order(created_at: :asc, project_id: :asc)
-        else
-          Ci::Build.pending.updated_before(lookback: BUILD_LOOKBACK.ago, timeout: timeout)
-        end
+        Ci::Build
+          .pending
+          .created_at_before(timeout)
+          .updated_at_before(timeout)
+          .order(created_at: :asc, project_id: :asc)
       end
       # rubocop: enable CodeReuse/ActiveRecord
     end

config/feature_flags/development/ci_new_query_for_pending_stuck_jobs.yml

----
-name: ci_new_query_for_pending_stuck_jobs
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68880
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339322
-milestone: '14.3'
-type: development
-group: group::pipeline execution
-default_enabled: false

spec/models/commit_status_spec.rb

@@ -97,32 +97,6 @@ RSpec.describe CommitStatus do
     end
   end
 
-  describe '.updated_before' do
-    let!(:lookback) { 5.days.ago }
-    let!(:timeout) { 1.day.ago }
-    let!(:before_lookback) { lookback - 1.hour }
-    let!(:after_lookback) { lookback + 1.hour }
-    let!(:before_timeout) { timeout - 1.hour }
-    let!(:after_timeout) { timeout + 1.hour }
-
-    subject { described_class.updated_before(lookback: lookback, timeout: timeout) }
-
-    def create_build_with_set_timestamps(created_at:, updated_at:)
-      travel_to(created_at) { create(:ci_build, created_at: Time.current) }.tap do |build|
-        travel_to(updated_at) { build.update!(status: :failed) }
-      end
-    end
-
-    it 'finds builds updated and created in the window between lookback and timeout' do
-      build_in_lookback_timeout_window = create_build_with_set_timestamps(created_at: after_lookback, updated_at: before_timeout)
-      build_outside_lookback_window = create_build_with_set_timestamps(created_at: before_lookback, updated_at: before_timeout)
-      build_outside_timeout_window = create_build_with_set_timestamps(created_at: after_lookback, updated_at: after_timeout)
-
-      expect(subject).to contain_exactly(build_in_lookback_timeout_window)
-      expect(subject).not_to include(build_outside_lookback_window, build_outside_timeout_window)
-    end
-  end
-
   describe '.scheduled_at_before' do
     let!(:never_scheduled) { create(:commit_status) }
     let!(:stale_scheduled) { create(:commit_status, scheduled_at: 1.day.ago) }

spec/services/ci/stuck_builds/drop_pending_service_spec.rb

@@ -3,8 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe Ci::StuckBuilds::DropPendingService do
-  let!(:runner) { create :ci_runner }
-  let!(:job) { create :ci_build, runner: runner }
+  let_it_be(:runner) { create(:ci_runner) }
+  let_it_be(:pipeline) { create(:ci_empty_pipeline) }
+  let_it_be_with_reload(:job) do
+    create(:ci_build, pipeline: pipeline, runner: runner)
+  end
+
   let(:created_at) { }
   let(:updated_at) { }
 
@@ -14,6 +18,8 @@ RSpec.describe Ci::StuckBuilds::DropPendingService do
     job_attributes = { status: status }
     job_attributes[:created_at] = created_at if created_at
     job_attributes[:updated_at] = updated_at if updated_at
+    job_attributes.compact!
+
     job.update!(job_attributes)
   end
 
@@ -41,12 +47,6 @@ RSpec.describe Ci::StuckBuilds::DropPendingService do
 
           it_behaves_like 'job is dropped with failure reason', 'stuck_or_timeout_failure'
         end
-
-        context 'when created_at is outside lookback window' do
-          let(:created_at) { described_class::BUILD_LOOKBACK - 1.day }
-
-          it_behaves_like 'job is unchanged'
-        end
       end
 
       context 'when job was updated less than 1 day ago' do
@@ -63,12 +63,6 @@ RSpec.describe Ci::StuckBuilds::DropPendingService do
 
           it_behaves_like 'job is unchanged'
         end
-
-        context 'when created_at is outside lookback window' do
-          let(:created_at) { described_class::BUILD_LOOKBACK - 1.day }
-
-          it_behaves_like 'job is unchanged'
-        end
       end
 
       context 'when job was updated more than 1 hour ago' do
@@ -85,12 +79,6 @@ RSpec.describe Ci::StuckBuilds::DropPendingService do
 
           it_behaves_like 'job is unchanged'
         end
-
-        context 'when created_at is outside lookback window' do
-          let(:created_at) { described_class::BUILD_LOOKBACK - 1.day }
-
-          it_behaves_like 'job is unchanged'
-        end
       end
     end
 
@@ -115,12 +103,6 @@ RSpec.describe Ci::StuckBuilds::DropPendingService do
 
           it_behaves_like 'job is dropped with failure reason', 'stuck_or_timeout_failure'
         end
-
-        context 'when created_at is outside lookback window' do
-          let(:created_at) { described_class::BUILD_LOOKBACK - 1.day }
-
-          it_behaves_like 'job is unchanged'
-        end
       end
 
       context 'when job was updated in less than 1 hour ago' do
@@ -137,12 +119,6 @@ RSpec.describe Ci::StuckBuilds::DropPendingService do
 
           it_behaves_like 'job is unchanged'
         end
-
-        context 'when created_at is outside lookback window' do
-          let(:created_at) { described_class::BUILD_LOOKBACK - 1.day }
-
-          it_behaves_like 'job is unchanged'
-        end
       end
     end
   end
@@ -179,12 +155,6 @@ RSpec.describe Ci::StuckBuilds::DropPendingService do
 
         it_behaves_like 'job is unchanged'
       end
-
-      context 'when created_at is outside lookback window' do
-        let(:created_at) { described_class::BUILD_LOOKBACK - 1.day }
-
-        it_behaves_like 'job is unchanged'
-      end
     end
   end