GEO: Error handling - Specs

app/services/geo/base_sync_service.rb

@@ -182,7 +182,7 @@ module Geo
 
     def build_temporary_repository
       gitlab_shell.add_repository(project.repository_storage, disk_path_temp)
-      repository.clone.tap{|repo| repo.disk_path = disk_path_temp}
+      repository.clone.tap { |repo| repo.disk_path = disk_path_temp }
     end
 
     def clean_up_temporary_repository

app/services/geo/repository_sync_service.rb

@@ -35,7 +35,8 @@ module Geo
     rescue Gitlab::Git::Repository::NoRepository => e
       log_error('Invalid repository', e)
       registry.update(force_to_redownload_repository: true)
-      expire_repository_caches
+      log_info('Expiring caches')
+      project.repository.after_create
     ensure
       clean_up_temporary_repository if redownload
     end

spec/models/geo/file_registry_spec.rb

@@ -15,4 +15,13 @@ describe Geo::FileRegistry do
       expect(described_class.synced).to contain_exactly(synced)
     end
   end
+
+  describe '.retry_due' do
+    set(:retry_yesterday) { create(:geo_file_registry, retry_at: Date.yesterday) }
+    set(:retry_tomorrow) { create(:geo_file_registry, retry_at: Date.tomorrow) }
+
+    it 'returns registries in the synced state' do
+      expect(described_class.retry_due).not_to contain_exactly([retry_tomorrow])
+    end
+  end
 end

spec/models/geo/project_registry_spec.rb

@@ -38,6 +38,16 @@ describe Geo::ProjectRegistry do
     end
   end
 
+  describe '.retry_due' do
+    it 'returns projects that should be synced' do
+      create(:geo_project_registry, repository_retry_at: Date.yesterday, wiki_retry_at: Date.yesterday)
+      tomorrow = create(:geo_project_registry, repository_retry_at: Date.tomorrow, wiki_retry_at: Date.tomorrow)
+      not_set_retry_at = create(:geo_project_registry)
+
+      expect(described_class.retry_due).not_to include(tomorrow)
+    end
+  end
+
   describe '#repository_sync_due?' do
     where(:resync_repository, :last_successful_sync, :last_sync, :expected) do
       now = Time.now

spec/services/geo/file_download_service_spec.rb

@@ -29,6 +29,8 @@ describe Geo::FileDownloadService do
         stub_transfer(Gitlab::Geo::FileTransfer, -1)
 
         expect { execute! }.to change { Geo::FileRegistry.failed.count }.by(1)
+        expect(Geo::FileRegistry.last.retry_count).to eq(1)
+        expect(Geo::FileRegistry.last.retry_at).to be_present
       end
 
       it 'registers when the download fails with some other error' do

spec/services/geo/repository_sync_service_spec.rb

 require 'spec_helper'
 
-RSpec.describe Geo::RepositorySyncService do
+describe Geo::RepositorySyncService do
   include ::EE::GeoHelpers
 
   set(:primary) { create(:geo_node, :primary, host: 'primary-geo-node', relative_url_root: '/gitlab') }
@@ -122,6 +122,15 @@ RSpec.describe Geo::RepositorySyncService do
 
           subject.execute
         end
+
+        it 'sets repository_retry_count and repository_retry_at to nil' do
+          registry = create(:geo_project_registry, project: project, repository_retry_count: 2, repository_retry_at: Date.yesterday)
+
+          subject.execute
+
+          expect(registry.reload.repository_retry_count).to be_nil
+          expect(registry.repository_retry_at).to be_nil
+        end
       end
 
       context 'when repository sync fail' do
@@ -140,6 +149,44 @@ RSpec.describe Geo::RepositorySyncService do
         it 'resets last_repository_successful_sync_at' do
           expect(registry.last_repository_successful_sync_at).to be_nil
         end
+
+        it 'resets repository_retry_count' do
+          expect(registry.repository_retry_count).to eq(1)
+        end
+
+        it 'resets repository_retry_at' do
+          expect(registry.repository_retry_at).to be_present
+        end
+      end
+    end
+
+    context 'retries' do
+      it 'tries to fetch repo' do
+        registry = create(:geo_project_registry, project: project, repository_retry_count: Geo::BaseSyncService::RETRY_BEFORE_REDOWNLOAD - 1)
+
+        expect_any_instance_of(Geo::RepositorySyncService).to receive(:fetch_project_repository).with(false)
+
+        subject.execute
+      end
+
+      it 'tries to redownload repo' do
+        registry = create(:geo_project_registry, project: project, repository_retry_count: Geo::BaseSyncService::RETRY_BEFORE_REDOWNLOAD + 1)
+
+        expect_any_instance_of(Geo::RepositorySyncService).to receive(:fetch_project_repository).with(true)
+
+        subject.execute
+      end
+
+      it 'tries to redownload repo when force_redownload flag is set' do
+        registry = create(:geo_project_registry,
+          project: project,
+          repository_retry_count: Geo::BaseSyncService::RETRY_BEFORE_REDOWNLOAD - 1,
+          force_to_redownload_repository: true
+        )
+
+        expect_any_instance_of(Geo::RepositorySyncService).to receive(:fetch_project_repository).with(true)
+
+        subject.execute
       end
     end
 

spec/workers/geo/file_download_dispatch_worker_spec.rb

@@ -96,7 +96,7 @@ describe Geo::FileDownloadDispatchWorker, :geo, :truncate do
     end
 
     context 'with a failed file' do
-      let!(:failed_registry) { create(:geo_file_registry, :lfs, file_id: 999, success: false) }
+      let(:failed_registry) { create(:geo_file_registry, :lfs, file_id: 999, success: false) }
 
       it 'does not stall backfill' do
         unsynced = create(:lfs_object, :with_file)
@@ -114,6 +114,22 @@ describe Geo::FileDownloadDispatchWorker, :geo, :truncate do
 
         subject.perform
       end
+
+      it 'does not retries failed files when retry_at is tomorrow' do
+        failed_registry = create(:geo_file_registry, :lfs, file_id: 999, success: false, retry_at: Date.tomorrow)
+
+        expect(GeoFileDownloadWorker).not_to receive(:perform_async).with('lfs', failed_registry.file_id)
+
+        subject.perform
+      end
+
+      it 'does not retries failed files when retry_at is in the past' do
+        failed_registry = create(:geo_file_registry, :lfs, file_id: 999, success: false, retry_at: Date.yesterday)
+
+        expect(GeoFileDownloadWorker).to receive(:perform_async).with('lfs', failed_registry.file_id)
+
+        subject.perform
+      end
     end
 
     context 'when node has namespace restrictions' do