Merge branch 'georgekoltsov-remove-sidekiq-import-retries' into 'master'

Disable sidekiq retries for Project/Group Import/Export

See merge request gitlab-org/gitlab!35344

app/workers/concerns/project_export_options.rb

-# frozen_string_literal: true
-
-module ProjectExportOptions
-  extend ActiveSupport::Concern
-
-  EXPORT_RETRY_COUNT = 3
-
-  included do
-    sidekiq_options retry: EXPORT_RETRY_COUNT, status_expiration: StuckExportJobsWorker::EXPORT_JOBS_EXPIRATION
-
-    # We mark the project export as failed once we have exhausted all retries
-    sidekiq_retries_exhausted do |job|
-      project = Project.find(job['args'][1])
-      # rubocop: disable CodeReuse/ActiveRecord
-      job = project.export_jobs.find_by(jid: job["jid"])
-      # rubocop: enable CodeReuse/ActiveRecord
-
-      if job&.fail_op
-        Sidekiq.logger.info "Job #{job['jid']} for project #{project.id} has been set to failed state"
-      else
-        Sidekiq.logger.error "Failed to set Job #{job['jid']} for project #{project.id} to failed state"
-      end
-    end
-  end
-end

app/workers/group_export_worker.rb

@@ -6,6 +6,7 @@ class GroupExportWorker # rubocop:disable Scalability/IdempotentWorker
 
   feature_category :importers
   loggable_arguments 2
+  sidekiq_options retry: false
 
   def perform(current_user_id, group_id, params = {})
     current_user = User.find(current_user_id)

app/workers/project_export_worker.rb

@@ -3,12 +3,13 @@
 class ProjectExportWorker # rubocop:disable Scalability/IdempotentWorker
   include ApplicationWorker
   include ExceptionBacktrace
-  include ProjectExportOptions
 
   feature_category :importers
   worker_resource_boundary :memory
   urgency :throttled
   loggable_arguments 2, 3
+  sidekiq_options retry: false
+  sidekiq_options status_expiration: StuckExportJobsWorker::EXPORT_JOBS_EXPIRATION
 
   def perform(current_user_id, project_id, after_export_strategy = {}, params = {})
     current_user = User.find(current_user_id)

app/workers/repository_import_worker.rb

@@ -4,10 +4,11 @@ class RepositoryImportWorker # rubocop:disable Scalability/IdempotentWorker
   include ApplicationWorker
   include ExceptionBacktrace
   include ProjectStartImport
-  include ProjectImportOptions
 
   feature_category :importers
   worker_has_external_dependencies!
+  sidekiq_options retry: false
+  sidekiq_options status_expiration: Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION
 
   # technical debt: https://gitlab.com/gitlab-org/gitlab/issues/33991
   sidekiq_options memory_killer_memory_growth_kb: ENV.fetch('MEMORY_KILLER_REPOSITORY_IMPORT_WORKER_MEMORY_GROWTH_KB', 50).to_i

ee/spec/workers/repository_import_worker_spec.rb

@@ -35,4 +35,14 @@ RSpec.describe RepositoryImportWorker do
       subject.perform(project.id)
     end
   end
+
+  describe 'sidekiq options' do
+    it 'disables retry' do
+      expect(described_class.sidekiq_options['retry']).to eq(false)
+    end
+
+    it 'sets default status expiration' do
+      expect(described_class.sidekiq_options['status_expiration']).to eq(Gitlab::Import::StuckImportJob::IMPORT_JOBS_EXPIRATION)
+    end
+  end
 end

spec/workers/concerns/project_export_options_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe ProjectExportOptions do
-  let(:project) { create(:project) }
-  let(:project_export_job) { create(:project_export_job, project: project, jid: '123', status: 1) }
-  let(:job) { { 'args' => [project.owner.id, project.id, nil, nil], 'jid' => '123' } }
-  let(:worker_class) do
-    Class.new do
-      include Sidekiq::Worker
-      include ProjectExportOptions
-    end
-  end
-
-  it 'sets default retry limit' do
-    expect(worker_class.sidekiq_options['retry']).to eq(ProjectExportOptions::EXPORT_RETRY_COUNT)
-  end
-
-  it 'sets default status expiration' do
-    expect(worker_class.sidekiq_options['status_expiration']).to eq(StuckExportJobsWorker::EXPORT_JOBS_EXPIRATION)
-  end
-
-  describe '.sidekiq_retries_exhausted' do
-    it 'marks status as failed' do
-      expect { worker_class.sidekiq_retries_exhausted_block.call(job) }.to change { project_export_job.reload.status }.from(1).to(3)
-    end
-
-    context 'when status update fails' do
-      before do
-        project_export_job.update(status: 2)
-      end
-
-      it 'logs an error' do
-        expect(Sidekiq.logger).to receive(:error).with("Failed to set Job #{job['jid']} for project #{project.id} to failed state")
-
-        worker_class.sidekiq_retries_exhausted_block.call(job)
-      end
-    end
-  end
-end

spec/workers/project_export_worker_spec.rb

@@ -69,4 +69,14 @@ RSpec.describe ProjectExportWorker do
       end
     end
   end
+
+  describe 'sidekiq options' do
+    it 'disables retry' do
+      expect(described_class.sidekiq_options['retry']).to eq(false)
+    end
+
+    it 'sets default status expiration' do
+      expect(described_class.sidekiq_options['status_expiration']).to eq(StuckExportJobsWorker::EXPORT_JOBS_EXPIRATION)
+    end
+  end
 end

spec/workers/repository_import_worker_spec.rb

@@ -3,12 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe RepositoryImportWorker do
-  describe 'modules' do
-    it 'includes ProjectImportOptions' do
-      expect(described_class).to include_module(ProjectImportOptions)
-    end
-  end
-
   describe '#perform' do
     let(:project) { create(:project, :import_scheduled) }
     let(:import_state) { project.import_state }