Merge branch 'da-fix-projects-backfill' into 'master'

Backfill projects where the last attempt failed

Closes #1965

See merge request !1785

app/services/geo/repository_backfill_service.rb

@@ -10,7 +10,17 @@ module Geo
     end
 
     def execute
-      return if backfilled?(project)
+      # When Geo customers upgrade to 9.0, the secondaries nodes that are
+      # enabled will start the backfilling process automatically. We need
+      # to populate the tracking database correctly for projects synced
+      # before the process being started or projects created during the
+      # backfilling. Otherwise, the query to retrieve the projects will
+      # always return the same projects because they don't have entries
+      # in the tracking database
+      if backfilled?
+        update_registry(DateTime.now, DateTime.now)
+        return
+      end
 
       try_obtain_lease do
         log('Started repository sync')
@@ -39,7 +49,7 @@ module Geo
 
         finished_at = DateTime.now
       rescue Gitlab::Shell::Error => e
-        Rails.logger.error "Error syncing repository for project #{project.path_with_namespace}: #{e}"
+        Rails.logger.error("#{self.class.name}: Error syncing repository for project #{project.path_with_namespace}: #{e}")
       end
 
       [started_at, finished_at]
@@ -83,32 +93,27 @@ module Geo
       Gitlab::ExclusiveLease.cancel(lease_key, repository_lease)
     end
 
-    def backfilled?(project)
+    def backfilled?
       return false unless project.repository.exists?
       return false if project.repository.exists? && project.repository.empty?
-      return true if registry_exists?(project)
-
-      # When Geo customers upgrade to 9.0, the secondaries nodes that are
-      # enabled will start the backfilling process automatically. We need
-      # to populate the tracking database correctly for projects synced
-      # before the process being started or projects created during the
-      # backfilling. Otherwise, the query to retrieve the projects will
-      # always return the same projects because they don't have entries
-      # in the tracking database
-      update_registry(DateTime.now, DateTime.now)
+      return false if failed_registry_exists?
 
       true
     end
 
-    def registry_exists?(project)
-      Geo::ProjectRegistry.where(project_id: project.id)
-                          .where.not(last_repository_synced_at: nil)
-                          .any?
+    def failed_registry_exists?
+      Geo::ProjectRegistry.failed.where(project_id: project_id).any?
+    end
+
+    def synced_registry_exists?
+      Geo::ProjectRegistry.synced.where(project_id: project_id).any?
     end
 
     def update_registry(started_at, finished_at)
+      return if synced_registry_exists?
+
       log('Updating registry information')
-      registry = Geo::ProjectRegistry.find_or_initialize_by(project_id: project.id)
+      registry = Geo::ProjectRegistry.find_or_initialize_by(project_id: project_id)
       registry.last_repository_synced_at = started_at
       registry.last_repository_successful_sync_at = finished_at if finished_at
       registry.save
@@ -131,7 +136,7 @@ module Geo
     end
 
     def log(message)
-      Rails.logger.info "#{self.class.name}: #{message} for project #{project.path_with_namespace} (#{project.id})"
+      Rails.logger.info("#{self.class.name}: #{message} for project #{project.path_with_namespace} (#{project.id})")
     end
   end
 end

app/workers/geo_backfill_worker.rb

@@ -39,7 +39,7 @@ class GeoBackfillWorker
   private
 
   def find_project_ids
-    Project.where.not(id: Geo::ProjectRegistry.pluck(:project_id))
+    Project.where.not(id: Geo::ProjectRegistry.synced.pluck(:project_id))
            .limit(BATCH_SIZE)
            .pluck(:id)
   end

changelogs/unreleased-ee/da-fix-projects-backfill.yml

+---
+title: Backfill projects where the last attempt to backfill failed
+merge_request:
+author:

spec/services/geo/repository_backfill_service_spec.rb

@@ -80,14 +80,15 @@ describe Geo::RepositoryBackfillService, services: true do
       end
     end
 
-    context 'when repository was backfilled' do
+    context 'when repository was backfilled successfully' do
       let(:project) { create(:project) }
+      let(:last_repository_successful_sync_at) { 5.days.ago }
 
       let!(:registry) do
         Geo::ProjectRegistry.create(
           project: project,
-          last_repository_synced_at: DateTime.now,
-          last_repository_successful_sync_at: DateTime.now
+          last_repository_synced_at: 5.days.ago,
+          last_repository_successful_sync_at: last_repository_successful_sync_at
         )
       end
 
@@ -98,9 +99,55 @@ describe Geo::RepositoryBackfillService, services: true do
       end
 
       context 'tracking database' do
-        it 'does not track repository sync' do
+        it 'does not create a new registry' do
           expect { subject.execute }.not_to change(Geo::ProjectRegistry, :count)
         end
+
+        it 'does not update last_repository_successful_sync_at' do
+          subject.execute
+
+          expect(registry.reload.last_repository_successful_sync_at).to be_within(1.second).of(last_repository_successful_sync_at)
+        end
+      end
+    end
+
+    context 'when last attempt to backfill the repository failed' do
+      let(:project) { create(:project) }
+
+      let!(:registry) do
+        Geo::ProjectRegistry.create(
+          project: project,
+          last_repository_synced_at: DateTime.now,
+          last_repository_successful_sync_at: nil
+        )
+      end
+
+      it 'fetches project repositories' do
+        fetch_count = 0
+
+        allow_any_instance_of(Repository).to receive(:fetch_geo_mirror) do
+          fetch_count += 1
+        end
+
+        subject.execute
+
+        expect(fetch_count).to eq 2
+      end
+
+      context 'tracking database' do
+        before do
+          allow_any_instance_of(Repository).to receive(:fetch_geo_mirror) { true }
+        end
+
+        it 'does not create a new registry' do
+          expect { subject.execute }.not_to change(Geo::ProjectRegistry, :count)
+        end
+
+        it 'updates last_repository_successful_sync_at' do
+          subject.execute
+
+          expect(registry.reload.last_repository_successful_sync_at).not_to be_nil
+        end
       end
     end
   end

spec/workers/geo/geo_backfill_worker_spec.rb

@@ -18,6 +18,18 @@ describe Geo::GeoBackfillWorker, services: true do
       subject.perform
     end
 
+    it 'performs Geo::RepositoryBackfillService for projects where last attempt to backfill failed' do
+      Geo::ProjectRegistry.create(
+        project: Project.first,
+        last_repository_synced_at: DateTime.now,
+        last_repository_successful_sync_at: nil
+      )
+
+      expect(Geo::RepositoryBackfillService).to receive(:new).twice.and_return(spy)
+
+      subject.perform
+    end
+
     it 'does not perform Geo::RepositoryBackfillService when tracking DB is not available' do
       allow(Rails.configuration).to receive(:respond_to?).with(:geo_database) { false }