Retry the failing projects

lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config.rb

@@ -12,7 +12,7 @@ module Gitlab
         class HashedProject
           attr_accessor :project
 
-          ROOT_PATH_PREFIX = '@hashed'.freeze
+          ROOT_PATH_PREFIX = '@hashed'
 
           def initialize(project)
             @project = project
@@ -42,7 +42,7 @@ module Gitlab
       end
 
       # Concern used by Project and Namespace to determine the full
-      # route the the project
+      # route to the project
       module Routable
         extend ActiveSupport::Concern
 
@@ -128,22 +128,65 @@ module Gitlab
         end
       end
 
-      # Class to add the fullpath to the git repo config
-      class Up
+      # Base class for Up and Down migration classes
+      class BackfillFullpathMigration
+        RETRY_DELAY = 15.minutes
+        MAX_RETRIES = 2
+
+        # Base class for retrying one project
+        class BaseRetryOne
+          def perform(project_id, retry_count)
+            project = Project.find(project_id)
+
+            migration_class.new.safe_perform_one(project, retry_count) if project
+          end
+        end
+
         def perform(start_id, end_id)
           Project.where(id: start_id..end_id).each do |project|
-            project.add_fullpath_config
+            safe_perform_one(project)
           end
         end
+
+        def safe_perform_one(project, retry_count = 0)
+          perform_one(project)
+        rescue GRPC::NotFound, GRPC::InvalidArgument
+          nil
+        rescue GRPC::BadStatus
+          schedule_retry(project, retry_count + 1) if retry_count < MAX_RETRIES
+        end
+
+        def schedule_retry(project, retry_count)
+          BackgroundMigrationWorker.perform_in(RETRY_DELAY, self.class::RetryOne.name, [project.id, retry_count])
+        end
+      end
+
+      # Class to add the fullpath to the git repo config
+      class Up < BackfillFullpathMigration
+        # Class used to retry
+        class RetryOne < BaseRetryOne
+          def migration_class
+            Up
+          end
+        end
+
+        def perform_one(project)
+          project.add_fullpath_config
+        end
       end
 
       # Class to rollback adding the fullpath to the git repo config
-      class Down
-        def perform(start_id, end_id)
-          Project.where(id: start_id..end_id).each do |project|
-            project.remove_fullpath_config
+      class Down < BackfillFullpathMigration
+        # Class used to retry
+        class RetryOne < BaseRetryOne
+          def migration_class
+            Down
           end
         end
+
+        def perform_one(project)
+          project.remove_fullpath_config
+        end
       end
     end
   end

spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::BackfillProjectFullpathInRepoConfig, :migration, schema: 20181010133639 do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:group) { namespaces.create!(name: 'foo', path: 'foo') }
+  let(:subgroup) { namespaces.create!(name: 'bar', path: 'bar', parent_id: group.id) }
+
+  describe described_class::Storage::HashedProject do
+    let(:project) { double(id: 555) }
+    subject(:project_storage) { described_class.new(project) }
+
+    it 'has the correct disk_path' do
+      expect(project_storage.disk_path).to eq('@hashed/91/a7/91a73fd806ab2c005c13b4dc19130a884e909dea3f72d46e30266fe1a1f588d8')
+    end
+  end
+
+  describe described_class::Storage::LegacyProject do
+    let(:project) { double(full_path: 'this/is/the/full/path') }
+    subject(:project_storage) { described_class.new(project) }
+
+    it 'has the correct disk_path' do
+      expect(project_storage.disk_path).to eq('this/is/the/full/path')
+    end
+  end
+
+  describe described_class::Project do
+    let(:project_record) { projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz') }
+    subject(:project) { described_class.find(project_record.id) }
+
+    describe '#full_path' do
+      it 'returns path containing all parent namespaces' do
+        expect(project.full_path).to eq('foo/bar/baz')
+      end
+    end
+  end
+
+  describe described_class::Up do
+    describe '#perform' do
+      subject(:migrate) { described_class.new.perform(projects.minimum(:id), projects.maximum(:id)) }
+
+      it 'asks the gitaly client to set config' do
+        projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz')
+        projects.create!(namespace_id: subgroup.id, name: 'buzz', path: 'buzz', storage_version: 1)
+
+        expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service|
+          expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => 'foo/bar/baz')
+        end
+
+        expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service|
+          expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => 'foo/bar/buzz')
+        end
+
+        migrate
+      end
+    end
+  end
+
+  describe described_class::Down do
+    describe '#perform' do
+      subject(:migrate) { described_class.new.perform(projects.minimum(:id), projects.maximum(:id)) }
+
+      it 'asks the gitaly client to set config' do
+        projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz')
+
+        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService)
+          .to receive(:delete_config).with(['gitlab.fullpath'])
+
+        migrate
+      end
+    end
+  end
+end

spec/migrations/backfill_store_project_full_path_in_repo_spec.rb

@@ -27,6 +27,16 @@ describe BackfillStoreProjectFullPathInRepo, :migration do
         migration.up
       end
 
+      it 'retries in case of failure' do
+        repository_service = spy(:repository_service)
+
+        allow(Gitlab::GitalyClient::RepositoryService).to receive(:new).and_return(repository_service)
+        allow(repository_service).to receive(:set_config).and_raise(GRPC::BadStatus, 'Retry me')
+        expect(repository_service).to receive(:set_config).exactly(3).times
+
+        migration.up
+      end
+
       context 'legacy storage' do
         it 'finds the repository at the correct location' do
           Project.find(project.id).create_repository