Merge branch 'eb-clean-up-unit-test-tables' into 'master'

Add cron worker for cleaning up unit test tables

See merge request gitlab-org/gitlab!61463

app/models/ci/unit_test.rb

@@ -14,6 +14,7 @@ module Ci
     belongs_to :project
 
     scope :by_project_and_keys, -> (project, keys) { where(project_id: project.id, key_hash: keys) }
+    scope :deletable, -> { where('NOT EXISTS (?)', Ci::UnitTestFailure.select(1).where("#{Ci::UnitTestFailure.table_name}.unit_test_id = #{table_name}.id")) }
 
     class << self
       def find_or_create_by_batch(project, unit_test_attrs)

app/models/ci/unit_test_failure.rb

@@ -11,6 +11,8 @@ module Ci
     belongs_to :unit_test, class_name: "Ci::UnitTest", foreign_key: :unit_test_id
     belongs_to :build, class_name: "Ci::Build", foreign_key: :build_id
 
+    scope :deletable, -> { where('failed_at < ?', REPORT_WINDOW.ago) }
+
     def self.recent_failures_count(project:, unit_test_keys:, date_range: REPORT_WINDOW.ago..Time.current)
       joins(:unit_test)
         .where(

app/services/ci/delete_unit_tests_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class DeleteUnitTestsService
+    include EachBatch
+
+    BATCH_SIZE = 100
+
+    def execute
+      purge_data!(Ci::UnitTestFailure)
+      purge_data!(Ci::UnitTest)
+    end
+
+    private
+
+    def purge_data!(klass)
+      loop do
+        break unless delete_batch!(klass)
+      end
+    end
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def delete_batch!(klass)
+      deleted = 0
+
+      ActiveRecord::Base.transaction do
+        ids = klass.deletable.lock('FOR UPDATE SKIP LOCKED').limit(BATCH_SIZE).pluck(:id)
+        break if ids.empty?
+
+        deleted = klass.where(id: ids).delete_all
+      end
+
+      deleted > 0
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+  end
+end

app/workers/all_queues.yml

@@ -182,6 +182,15 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: cronjob:ci_delete_unit_tests
+  :worker_name: Ci::DeleteUnitTestsWorker
+  :feature_category: :continuous_integration
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: cronjob:ci_pipeline_artifacts_expire_artifacts
   :worker_name: Ci::PipelineArtifacts::ExpireArtifactsWorker
   :feature_category: :continuous_integration

app/workers/ci/delete_unit_tests_worker.rb

+# frozen_string_literal: true
+
+module Ci
+  class DeleteUnitTestsWorker
+    include ApplicationWorker
+    # rubocop:disable Scalability/CronWorkerContext
+    # This worker does not perform work scoped to a context
+    include CronjobQueue
+    # rubocop:enable Scalability/CronWorkerContext
+
+    feature_category :continuous_integration
+    idempotent!
+
+    def perform
+      Ci::DeleteUnitTestsService.new.execute
+    end
+  end
+end

changelogs/unreleased/eb-clean-up-unit-test-tables.yml

+---
+title: Add cron worker for cleaning up unit test tables
+merge_request: 61463
+author:
+type: added

config/initializers/1_settings.rb

@@ -572,6 +572,9 @@ Settings.cron_jobs['ssh_keys_expiring_soon_notification_worker']['job_class'] =
 Settings.cron_jobs['users_deactivate_dormant_users_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['users_deactivate_dormant_users_worker']['cron'] ||= '21,42 0-4 * * *'
 Settings.cron_jobs['users_deactivate_dormant_users_worker']['job_class'] = 'Users::DeactivateDormantUsersWorker'
+Settings.cron_jobs['ci_delete_unit_tests_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['ci_delete_unit_tests_worker']['cron'] ||= '0 0 * * *'
+Settings.cron_jobs['ci_delete_unit_tests_worker']['job_class'] = 'Ci::DeleteUnitTestsWorker'
 
 Gitlab.com do
   Settings.cron_jobs['batched_background_migrations_worker'] ||= Settingslogic.new({})

spec/services/ci/delete_unit_tests_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::DeleteUnitTestsService do
+  describe '#execute' do
+    let!(:unit_test_1) { create(:ci_unit_test) }
+    let!(:unit_test_2) { create(:ci_unit_test) }
+    let!(:unit_test_3) { create(:ci_unit_test) }
+    let!(:unit_test_4) { create(:ci_unit_test) }
+    let!(:unit_test_1_recent_failure) { create(:ci_unit_test_failure, unit_test: unit_test_1) }
+    let!(:unit_test_1_old_failure) { create(:ci_unit_test_failure, unit_test: unit_test_1, failed_at: 15.days.ago) }
+    let!(:unit_test_2_old_failure) { create(:ci_unit_test_failure, unit_test: unit_test_2, failed_at: 15.days.ago) }
+    let!(:unit_test_3_old_failure) { create(:ci_unit_test_failure, unit_test: unit_test_3, failed_at: 15.days.ago) }
+    let!(:unit_test_4_old_failure) { create(:ci_unit_test_failure, unit_test: unit_test_4, failed_at: 15.days.ago) }
+
+    before do
+      stub_const("#{described_class.name}::BATCH_SIZE", 2)
+
+      described_class.new.execute
+    end
+
+    it 'does not delete unit test failures not older than 14 days' do
+      expect(unit_test_1_recent_failure.reload).to be_persisted
+    end
+
+    it 'deletes unit test failures older than 14 days' do
+      ids = [
+        unit_test_1_old_failure,
+        unit_test_2_old_failure,
+        unit_test_3_old_failure,
+        unit_test_4_old_failure
+      ].map(&:id)
+
+      result = Ci::UnitTestFailure.where(id: ids)
+
+      expect(result).to be_empty
+    end
+
+    it 'deletes unit tests that have no more associated unit test failures' do
+      ids = [
+        unit_test_2,
+        unit_test_3,
+        unit_test_4
+      ].map(&:id)
+
+      result = Ci::UnitTest.where(id: ids)
+
+      expect(result).to be_empty
+    end
+  end
+end

spec/workers/ci/delete_unit_tests_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::DeleteUnitTestsWorker do
+  let(:worker) { described_class.new }
+
+  describe '#perform' do
+    it 'executes a service' do
+      expect_next_instance_of(Ci::DeleteUnitTestsService) do |instance|
+        expect(instance).to receive(:execute)
+      end
+
+      worker.perform
+    end
+  end
+
+  it_behaves_like 'an idempotent worker' do
+    let!(:unit_test_1) { create(:ci_unit_test) }
+    let!(:unit_test_2) { create(:ci_unit_test) }
+    let!(:unit_test_1_recent_failure) { create(:ci_unit_test_failure, unit_test: unit_test_1) }
+    let!(:unit_test_2_old_failure) { create(:ci_unit_test_failure, unit_test: unit_test_2, failed_at: 15.days.ago) }
+
+    it 'only deletes old unit tests and their failures' do
+      subject
+
+      expect(unit_test_1.reload).to be_persisted
+      expect(unit_test_1_recent_failure.reload).to be_persisted
+      expect(Ci::UnitTest.find_by(id: unit_test_2.id)).to be_nil
+      expect(Ci::UnitTestFailure.find_by(id: unit_test_2_old_failure.id)).to be_nil
+    end
+  end
+end