From c76c514fba095e87e723ac08c8c4b0f8d6486d25 Mon Sep 17 00:00:00 2001
From: nmilojevic1 <nmilojevic@gitlab.com>
Date: Tue, 28 Jan 2020 10:59:45 +0100
Subject: [PATCH] Add retry mechanisam to after import
- use import_failure_service
- add missing specs
---
app/services/projects/after_import_service.rb | 8 ++-
.../projects/after_import_service_spec.rb | 49 ++++++++++++++++++-
2 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/app/services/projects/after_import_service.rb b/app/services/projects/after_import_service.rb
index 6fc15db9b4cfd..a2270f1054778 100644
--- a/app/services/projects/after_import_service.rb
+++ b/app/services/projects/after_import_service.rb
@@ -12,7 +12,9 @@ def execute
service = Projects::HousekeepingService.new(@project)
service.execute do
- repository.delete_all_refs_except(RESERVED_REF_PREFIXES)
+ import_failure_service.with_retry(action: 'delete_all_refs') do
+ repository.delete_all_refs_except(RESERVED_REF_PREFIXES)
+ end
end
# Right now we don't actually have a way to know if a project
@@ -26,6 +28,10 @@ def execute
private
+ def import_failure_service
+ @import_failure_service ||= Gitlab::ImportExport::ImportFailureService.new(@project)
+ end
+
def repository
@repository ||= @project.repository
end
diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb
index 27e8f3c45ba27..4830e71224de4 100644
--- a/spec/services/projects/after_import_service_spec.rb
+++ b/spec/services/projects/after_import_service_spec.rb
@@ -20,7 +20,7 @@
allow(housekeeping_service)
.to receive(:execute).and_yield
- expect(housekeeping_service).to receive(:increment!)
+ allow(housekeeping_service).to receive(:increment!)
end
it 'performs housekeeping' do
@@ -58,6 +58,53 @@
end
end
+ context 'when after import action throw non-retriable exception' do
+ let(:exception) { StandardError.new('after import error') }
+
+ before do
+ allow(repository)
+ .to receive(:delete_all_refs_except)
+ .and_raise(exception)
+ end
+
+ it 'throws after import error' do
+ expect { subject.execute }.to raise_exception('after import error')
+ end
+ end
+
+ context 'when after import action throw retriable exception one time' do
+ let(:exception) { GRPC::DeadlineExceeded.new }
+
+ before do
+ repository.write_ref('refs/pull/1/head', sha)
+ expect(repository)
+ .to receive(:delete_all_refs_except)
+ .and_raise(exception)
+ expect(repository)
+ .to receive(:delete_all_refs_except)
+ .and_call_original
+
+ subject.execute
+ end
+
+ it 'removes refs/pull/**/*' do
+ expect(rugged.references.map(&:name))
+ .not_to include(%r{\Arefs/pull/})
+ end
+
+ it 'records the failures in the database', :aggregate_failures do
+ import_failure = ImportFailure.last
+
+ expect(import_failure.source).to eq('delete_all_refs')
+ expect(import_failure.project_id).to eq(project.id)
+ expect(import_failure.relation_key).to be_nil
+ expect(import_failure.relation_index).to be_nil
+ expect(import_failure.exception_class).to eq('GRPC::DeadlineExceeded')
+ expect(import_failure.exception_message).to be_present
+ expect(import_failure.correlation_id_value).not_to be_empty
+ end
+ end
+
def rugged
rugged_repo(repository)
end
--
GitLab