Commit 095a6655 authored by George Koltsov's avatar George Koltsov

Move Project Export of templates into a separate sidekiq queue

- Fix an issue of delayed or failed project creation when using
  group level custom templates. Since it's using Project Export
  under the hood, regular project export requests can delay
  this type of project creation.
- In order to speed this process up, move Template Export into
  a separate worker/queue
parent b3d243da
......@@ -250,6 +250,8 @@
- 1
- - project_service
- 1
- - project_template_export
- 1
- - project_update_repository_storage
- 1
- - prometheus_create_default_alerts
......
......@@ -725,6 +725,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
......
......@@ -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
......
......@@ -733,6 +733,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:
......
# 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
---
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
......@@ -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
......@@ -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
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ProjectTemplateExportWorker do
it_behaves_like 'export worker'
end
# 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
......@@ -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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment