Merge branch 'georgekoltsov/move-template-export-to-separate-queue' into 'master'

Move Project Export of templates into a separate sidekiq queue

See merge request gitlab-org/gitlab!48134

config/sidekiq_queues.yml

@@ -250,6 +250,8 @@
   - 1
 - - project_service
   - 1
+- - project_template_export
+  - 1
 - - project_update_repository_storage
   - 1
 - - prometheus_create_default_alerts

ee/app/models/ee/project.rb

@@ -726,6 +726,16 @@ module EE
       super.concat(requirements_ci_variables)
     end
 
+    def add_template_export_job(current_user:, after_export_strategy: nil, params: {})
+      job_id = ProjectTemplateExportWorker.perform_async(current_user.id, self.id, after_export_strategy, params)
+
+      if job_id
+        ::Gitlab::AppLogger.info(message: 'Template Export job started', project_id: self.id, job_id: job_id)
+      else
+        ::Gitlab::AppLogger.error(message: 'Template Export job failed to start', project_id: self.id)
+      end
+    end
+
     private
 
     def group_hooks

ee/app/services/ee/projects/gitlab_projects_import_service.rb

@@ -10,8 +10,8 @@ module EE
       def execute
         super.tap do |project|
           if project.saved? && custom_template
-            custom_template.add_export_job(current_user: current_user,
-                                           after_export_strategy: export_strategy(project))
+            custom_template.add_template_export_job(current_user: current_user,
+                                                    after_export_strategy: export_strategy(project))
           end
         end
       end

ee/app/workers/all_queues.yml

@@ -741,6 +741,14 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: project_template_export
+  :feature_category: :templates
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent:
+  :tags: []
 - :name: refresh_license_compliance_checks
   :feature_category: :license_compliance
   :has_external_dependencies:

ee/app/workers/project_template_export_worker.rb

+# frozen_string_literal: true
+
+# ProjectTemplateExportWorker is identical to ProjectExportWorker
+# with the only exception of having higher urgency (low instead of throttled)
+# and separate queue in order to allow users to create projects
+# from custom templates faster, without getting stuck in the queue,
+# since project_export queue can get congested by export requests
+# which significantly delays project creation from custom templates.
+class ProjectTemplateExportWorker < ProjectExportWorker # rubocop:disable Scalability/IdempotentWorker
+  feature_category :templates
+  loggable_arguments 2, 3
+  sidekiq_options retry: false, dead: false
+  sidekiq_options status_expiration: StuckExportJobsWorker::EXPORT_JOBS_EXPIRATION
+end

ee/changelogs/unreleased/georgekoltsov-move-template-export-to-separate-queue.yml

+---
+title: Move Project Export of templates into a separate sidekiq queue in order to
+  make project creation from group level custom templates faster
+merge_request: 48134
+author:
+type: changed

ee/spec/models/project_spec.rb

@@ -2626,4 +2626,15 @@ RSpec.describe Project do
       project.mark_primary_write_location
     end
   end
+
+  describe '#add_template_export_job' do
+    it 'starts project template export job' do
+      user = create(:user)
+      project = build(:project)
+
+      expect(ProjectTemplateExportWorker).to receive(:perform_async).with(user.id, project.id, nil, {})
+
+      project.add_template_export_job(current_user: user)
+    end
+  end
 end

ee/spec/services/projects/gitlab_projects_import_service_spec.rb

@@ -16,9 +16,9 @@ RSpec.describe Projects::GitlabProjectsImportService do
   end
 
   describe '#execute' do
-    context 'creates export job' do
+    context 'creates template export job' do
       it 'if project saved and custom template exists' do
-        expect(custom_template).to receive(:add_export_job)
+        expect(custom_template).to receive(:add_template_export_job)
 
         project = subject.execute
 
@@ -27,8 +27,8 @@ RSpec.describe Projects::GitlabProjectsImportService do
 
       it 'sets custom template import strategy after export' do
         expect(custom_template)
-          .to receive(:add_export_job).with(current_user: namespace.owner,
-                                            after_export_strategy: instance_of(EE::Gitlab::ImportExport::AfterExportStrategies::CustomTemplateExportImportStrategy))
+          .to receive(:add_template_export_job).with(current_user: namespace.owner,
+                                                     after_export_strategy: instance_of(EE::Gitlab::ImportExport::AfterExportStrategies::CustomTemplateExportImportStrategy))
 
         subject.execute
       end
@@ -40,7 +40,7 @@ RSpec.describe Projects::GitlabProjectsImportService do
           allow(instance).to receive(:saved?).and_return(false)
         end
 
-        expect(custom_template).not_to receive(:add_export_job)
+        expect(custom_template).not_to receive(:add_template_export_job)
 
         project = subject.execute
 

ee/spec/workers/project_template_export_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ProjectTemplateExportWorker do
+  it_behaves_like 'export worker'
+end

spec/support/shared_examples/workers/project_export_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'export worker' do
+  describe '#perform' do
+    let!(:user) { create(:user) }
+    let!(:project) { create(:project) }
+
+    before do
+      allow_next_instance_of(described_class) do |job|
+        allow(job).to receive(:jid).and_return(SecureRandom.hex(8))
+      end
+    end
+
+    context 'when it succeeds' do
+      it 'calls the ExportService' do
+        expect_next_instance_of(::Projects::ImportExport::ExportService) do |service|
+          expect(service).to receive(:execute)
+        end
+
+        subject.perform(user.id, project.id, { 'klass' => 'Gitlab::ImportExport::AfterExportStrategies::DownloadNotificationStrategy' })
+      end
+
+      context 'export job' do
+        before do
+          allow_next_instance_of(::Projects::ImportExport::ExportService) do |service|
+            allow(service).to receive(:execute)
+          end
+        end
+
+        it 'creates an export job record for the project' do
+          expect { subject.perform(user.id, project.id, {}) }.to change { project.export_jobs.count }.from(0).to(1)
+        end
+
+        it 'sets the export job status to started' do
+          expect_next_instance_of(ProjectExportJob) do |job|
+            expect(job).to receive(:start)
+          end
+
+          subject.perform(user.id, project.id, {})
+        end
+
+        it 'sets the export job status to finished' do
+          expect_next_instance_of(ProjectExportJob) do |job|
+            expect(job).to receive(:finish)
+          end
+
+          subject.perform(user.id, project.id, {})
+        end
+      end
+    end
+
+    context 'when it fails' do
+      it 'does not raise an exception when strategy is invalid' do
+        expect(::Projects::ImportExport::ExportService).not_to receive(:new)
+
+        expect { subject.perform(user.id, project.id, { 'klass' => 'Whatever' }) }.not_to raise_error
+      end
+
+      it 'does not raise error when project cannot be found' do
+        expect { subject.perform(user.id, non_existing_record_id, {}) }.not_to raise_error
+      end
+
+      it 'does not raise error when user cannot be found' do
+        expect { subject.perform(non_existing_record_id, project.id, {}) }.not_to raise_error
+      end
+    end
+  end
+
+  describe 'sidekiq options' do
+    it 'disables retry' do
+      expect(described_class.sidekiq_options['retry']).to eq(false)
+    end
+
+    it 'disables dead' do
+      expect(described_class.sidekiq_options['dead']).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/project_export_worker_spec.rb

@@ -3,84 +3,5 @@
 require 'spec_helper'
 
 RSpec.describe ProjectExportWorker do
-  let!(:user) { create(:user) }
-  let!(:project) { create(:project) }
-
-  subject { described_class.new }
-
-  describe '#perform' do
-    before do
-      allow_next_instance_of(described_class) do |job|
-        allow(job).to receive(:jid).and_return(SecureRandom.hex(8))
-      end
-    end
-
-    context 'when it succeeds' do
-      it 'calls the ExportService' do
-        expect_next_instance_of(::Projects::ImportExport::ExportService) do |service|
-          expect(service).to receive(:execute)
-        end
-
-        subject.perform(user.id, project.id, { 'klass' => 'Gitlab::ImportExport::AfterExportStrategies::DownloadNotificationStrategy' })
-      end
-
-      context 'export job' do
-        before do
-          allow_next_instance_of(::Projects::ImportExport::ExportService) do |service|
-            allow(service).to receive(:execute)
-          end
-        end
-
-        it 'creates an export job record for the project' do
-          expect { subject.perform(user.id, project.id, {}) }.to change { project.export_jobs.count }.from(0).to(1)
-        end
-
-        it 'sets the export job status to started' do
-          expect_next_instance_of(ProjectExportJob) do |job|
-            expect(job).to receive(:start)
-          end
-
-          subject.perform(user.id, project.id, {})
-        end
-
-        it 'sets the export job status to finished' do
-          expect_next_instance_of(ProjectExportJob) do |job|
-            expect(job).to receive(:finish)
-          end
-
-          subject.perform(user.id, project.id, {})
-        end
-      end
-    end
-
-    context 'when it fails' do
-      it 'does not raise an exception when strategy is invalid' do
-        expect(::Projects::ImportExport::ExportService).not_to receive(:new)
-
-        expect { subject.perform(user.id, project.id, { 'klass' => 'Whatever' }) }.not_to raise_error
-      end
-
-      it 'does not raise error when project cannot be found' do
-        expect { subject.perform(user.id, non_existing_record_id, {}) }.not_to raise_error
-      end
-
-      it 'does not raise error when user cannot be found' do
-        expect { subject.perform(non_existing_record_id, project.id, {}) }.not_to raise_error
-      end
-    end
-  end
-
-  describe 'sidekiq options' do
-    it 'disables retry' do
-      expect(described_class.sidekiq_options['retry']).to eq(false)
-    end
-
-    it 'disables dead' do
-      expect(described_class.sidekiq_options['dead']).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
+  it_behaves_like 'export worker'
 end