Merge branch...

Merge branch '342919-on-gitlab-com-the-projectexportworker-sidekiq-job-has-about-a-12-failure-rate' into 'master'

Add retry to ProjectExportWorker

See merge request gitlab-org/gitlab!72536

app/services/projects/import_export/export_service.rb

@@ -36,6 +36,7 @@ module Projects
       private
 
       attr_accessor :shared
+      attr_reader :logger
 
       def execute_after_export_action(after_export_strategy)
         return unless after_export_strategy
@@ -74,7 +75,11 @@ module Projects
       end
 
       def project_tree_saver
-        tree_saver_class.new(project: project, current_user: current_user, shared: shared, params: params)
+        tree_saver_class.new(project: project,
+                             current_user: current_user,
+                             shared: shared,
+                             params: params,
+                             logger: logger)
       end
 
       def tree_saver_class
@@ -116,7 +121,7 @@ module Projects
       end
 
       def notify_success
-        @logger.info(
+        logger.info(
           message: 'Project successfully exported',
           project_name: project.name,
           project_id: project.id
@@ -124,7 +129,7 @@ module Projects
       end
 
       def notify_error
-        @logger.error(
+        logger.error(
           message: 'Project export error',
           export_errors: shared.errors.join(', '),
           project_name: project.name,

lib/gitlab/import_export/project/tree_saver.rb

@@ -6,20 +6,16 @@ module Gitlab
       class TreeSaver
         attr_reader :full_path
 
-        def initialize(project:, current_user:, shared:, params: {})
+        def initialize(project:, current_user:, shared:, params: {}, logger: Gitlab::Import::Logger)
           @params       = params
           @project      = project
           @current_user = current_user
           @shared       = shared
+          @logger       = logger
         end
 
         def save
-          ImportExport::Json::StreamingSerializer.new(
-            exportable,
-            reader.project_tree,
-            json_writer,
-            exportable_path: "project"
-          ).execute
+          stream_export
 
           true
         rescue StandardError => e
@@ -31,6 +27,32 @@ module Gitlab
 
         private
 
+        def stream_export
+          on_retry = proc do |exception, try, elapsed_time, next_interval|
+            @logger.info(
+              message: "Project export retry triggered from streaming",
+              'error.class': exception.class,
+              'error.message': exception.message,
+              try_count: try,
+              elapsed_time_s: elapsed_time,
+              wait_to_retry_s: next_interval,
+              project_name: @project.name,
+              project_id: @project.id
+            )
+          end
+
+          serializer = ImportExport::Json::StreamingSerializer.new(
+            exportable,
+            reader.project_tree,
+            json_writer,
+            exportable_path: "project"
+          )
+
+          Retriable.retriable(on: Net::OpenTimeout, on_retry: on_retry) do
+            serializer.execute
+          end
+        end
+
         def reader
           @reader ||= Gitlab::ImportExport::Reader.new(shared: @shared)
         end

spec/lib/gitlab/import_export/project/tree_saver_spec.rb

@@ -5,6 +5,9 @@ require 'spec_helper'
 RSpec.describe Gitlab::ImportExport::Project::TreeSaver do
   let_it_be(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" }
   let_it_be(:exportable_path) { 'project' }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { setup_project }
 
   shared_examples 'saves project tree successfully' do |ndjson_enabled|
     include ImportExport::CommonUtil
@@ -12,9 +15,6 @@ RSpec.describe Gitlab::ImportExport::Project::TreeSaver do
     subject { get_json(full_path, exportable_path, relation_name, ndjson_enabled) }
 
     describe 'saves project tree attributes' do
-      let_it_be(:user) { create(:user) }
-      let_it_be(:group) { create(:group) }
-      let_it_be(:project) { setup_project }
       let_it_be(:shared) { project.import_export_shared }
 
       let(:relation_name) { :projects }
@@ -402,6 +402,50 @@ RSpec.describe Gitlab::ImportExport::Project::TreeSaver do
     it_behaves_like "saves project tree successfully", true
   end
 
+  context 'when streaming has to retry', :aggregate_failures do
+    let(:shared) { double('shared', export_path: exportable_path) }
+    let(:logger) { Gitlab::Import::Logger.build }
+    let(:serializer) { double('serializer') }
+    let(:error_class) { Net::OpenTimeout }
+    let(:info_params) do
+      {
+        'error.class': error_class,
+        project_name: project.name,
+        project_id: project.id
+      }
+    end
+
+    before do
+      allow(Gitlab::ImportExport::Json::StreamingSerializer).to receive(:new).and_return(serializer)
+    end
+
+    subject(:project_tree_saver) do
+      described_class.new(project: project, current_user: user, shared: shared, logger: logger)
+    end
+
+    it 'retries and succeeds' do
+      call_count = 0
+      allow(serializer).to receive(:execute) do
+        call_count += 1
+        call_count > 1 ? true : raise(error_class, 'execution expired')
+      end
+
+      expect(logger).to receive(:info).with(hash_including(info_params)).once
+
+      expect(project_tree_saver.save).to be(true)
+    end
+
+    it 'retries and does not succeed' do
+      retry_count = 3
+      allow(serializer).to receive(:execute).and_raise(error_class, 'execution expired')
+
+      expect(logger).to receive(:info).with(hash_including(info_params)).exactly(retry_count).times
+      expect(shared).to receive(:error).with(instance_of(error_class))
+
+      expect(project_tree_saver.save).to be(false)
+    end
+  end
+
   def setup_project
     release = create(:release)
 

spec/services/projects/import_export/export_service_spec.rb

@@ -4,7 +4,8 @@ require 'spec_helper'
 
 RSpec.describe Projects::ImportExport::ExportService do
   describe '#execute' do
-    let!(:user) { create(:user) }
+    let_it_be(:user) { create(:user) }
+
     let(:project) { create(:project) }
     let(:shared) { project.import_export_shared }
     let!(:after_export_strategy) { Gitlab::ImportExport::AfterExportStrategies::DownloadNotificationStrategy.new }
@@ -28,7 +29,14 @@ RSpec.describe Projects::ImportExport::ExportService do
     end
 
     it 'saves the models' do
-      expect(Gitlab::ImportExport::Project::TreeSaver).to receive(:new).and_call_original
+      saver_params = {
+        project: project,
+        current_user: user,
+        shared: shared,
+        params: {},
+        logger: an_instance_of(Gitlab::Export::Logger)
+      }
+      expect(Gitlab::ImportExport::Project::TreeSaver).to receive(:new).with(saver_params).and_call_original
 
       service.execute
     end

spec/support/retriable.rb

+# frozen_string_literal: true
+
+Retriable.configure do |config|
+  config.multiplier    = 1.0
+  config.rand_factor   = 0.0
+  config.base_interval = 0
+end