Store the time the sync started before fetching the repositories

app/services/geo/repository_sync_service.rb

@@ -33,8 +33,7 @@ module Geo
     def sync_project_repository
       return unless sync_repository?
 
-      started_at, finished_at = fetch_project_repository
-      update_registry(:repository, started_at, finished_at)
+      fetch_project_repository
       expire_repository_caches
     end
 
@@ -47,8 +46,7 @@ module Geo
     def sync_wiki_repository
       return unless sync_wiki?
 
-      started_at, finished_at = fetch_wiki_repository
-      update_registry(:wiki, started_at, finished_at)
+      fetch_wiki_repository
     end
 
     def sync_wiki?
@@ -58,17 +56,14 @@ module Geo
     end
 
     def fetch_project_repository
-      return unless sync_repository?
-
       log('Fetching project repository')
-      started_at  = DateTime.now
-      finished_at = nil
+      update_registry(:repository, started_at: DateTime.now)
 
       begin
         project.ensure_repository
         project.repository.fetch_geo_mirror(ssh_url_to_repo)
 
-        finished_at = DateTime.now
+        update_registry(:repository, finished_at: DateTime.now)
       rescue Gitlab::Shell::Error => e
         Rails.logger.error("#{self.class.name}: Error syncing repository for project #{project.path_with_namespace}: #{e}")
       rescue Gitlab::Git::Repository::NoRepository => e
@@ -76,27 +71,20 @@ module Geo
         log('Expiring caches')
         project.repository.after_create
       end
-
-      [started_at, finished_at]
     end
 
     def fetch_wiki_repository
-      return unless sync_wiki?
-
       log('Fetching wiki repository')
-      started_at  = DateTime.now
-      finished_at = nil
+      update_registry(:wiki, started_at: DateTime.now)
 
       begin
         project.wiki.ensure_repository
         project.wiki.repository.fetch_geo_mirror(ssh_url_to_wiki)
 
-        finished_at = DateTime.now
+        update_registry(:wiki, finished_at: DateTime.now)
       rescue Gitlab::Git::Repository::NoRepository, Gitlab::Shell::Error, ProjectWiki::CouldNotCreateWikiError => e
         Rails.logger.error("#{self.class.name}: Error syncing wiki repository for project #{project.path_with_namespace}: #{e}")
       end
-
-      [started_at, finished_at]
     end
 
     def expire_repository_caches
@@ -122,9 +110,14 @@ module Geo
       Gitlab::ExclusiveLease.cancel(lease_key, repository_lease)
     end
 
-    def update_registry(type, started_at, finished_at)
+    def update_registry(type, started_at: nil, finished_at: nil)
+      return unless started_at || finished_at
+
       log("Updating #{type} sync information")
-      registry.public_send("last_#{type}_synced_at=", started_at)
+
+      if started_at
+        registry.public_send("last_#{type}_synced_at=", started_at)
+      end
 
       if finished_at
         registry.public_send("last_#{type}_successful_sync_at=", finished_at)

app/workers/geo_repository_sync_worker.rb

@@ -30,6 +30,7 @@ class GeoRepositorySyncWorker
         try_obtain_lease do |lease|
           Geo::RepositorySyncService.new(project_id).execute
         end
+
       rescue ActiveRecord::RecordNotFound
         logger.error("Couldn't find project with ID=#{project_id}, skipping syncing")
         next

spec/services/geo/repository_sync_service_spec.rb

@@ -59,40 +59,72 @@ describe Geo::RepositorySyncService, services: true do
           expect { subject.execute }.to change(Geo::ProjectRegistry, :count).by(1)
         end
 
-        it 'sets last_repository_successful_sync_at when repository sync succeed' do
-          subject.execute
+        context 'when repository sync succeed' do
+          let(:registry) { Geo::ProjectRegistry.find_by(project_id: project.id) }
 
-          registry = Geo::ProjectRegistry.find_by(project_id: project.id)
+          before do
+            subject.execute
+          end
 
-          expect(registry.last_repository_successful_sync_at).not_to be_nil
+          it 'sets last_repository_synced_at' do
+            expect(registry.last_repository_synced_at).not_to be_nil
+          end
+
+          it 'sets last_repository_successful_sync_at' do
+            expect(registry.last_repository_successful_sync_at).not_to be_nil
+          end
         end
 
-        it 'resets last_repository_successful_sync_at when repository sync fail' do
-          allow_any_instance_of(Repository).to receive(:fetch_geo_mirror).with(/#{project.path_with_namespace}\.git/) { raise Gitlab::Shell::Error }
+        context 'when repository sync fail' do
+          let(:registry) { Geo::ProjectRegistry.find_by(project_id: project.id) }
 
-          subject.execute
+          before do
+            allow_any_instance_of(Repository).to receive(:fetch_geo_mirror).with(/#{project.path_with_namespace}\.git/) { raise Gitlab::Shell::Error }
+
+            subject.execute
+          end
 
-          registry = Geo::ProjectRegistry.find_by(project_id: project.id)
+          it 'sets last_repository_synced_at' do
+            expect(registry.last_repository_synced_at).not_to be_nil
+          end
 
-          expect(registry.last_repository_successful_sync_at).to be_nil
+          it 'resets last_repository_successful_sync_at' do
+            expect(registry.last_repository_successful_sync_at).to be_nil
+          end
         end
 
-        it 'sets last_wiki_successful_sync_at when wiki sync succeed' do
-          subject.execute
+        context 'when wiki sync succeed' do
+          let(:registry) { Geo::ProjectRegistry.find_by(project_id: project.id) }
 
-          registry = Geo::ProjectRegistry.find_by(project_id: project.id)
+          before do
+            subject.execute
+          end
 
-          expect(registry.last_wiki_successful_sync_at).not_to be_nil
+          it 'sets last_wiki_synced_at' do
+            expect(registry.last_wiki_synced_at).not_to be_nil
+          end
+
+          it 'sets last_wiki_successful_sync_at' do
+            expect(registry.last_wiki_successful_sync_at).not_to be_nil
+          end
         end
 
-        it 'resets last_wiki_successful_sync_at when wiki sync fail' do
-          allow_any_instance_of(Repository).to receive(:fetch_geo_mirror).with(/#{project.path_with_namespace}\.wiki.git/) { raise Gitlab::Shell::Error }
+        context 'when wiki sync fail' do
+          let(:registry) { Geo::ProjectRegistry.find_by(project_id: project.id) }
 
-          subject.execute
+          before do
+            allow_any_instance_of(Repository).to receive(:fetch_geo_mirror).with(/#{project.path_with_namespace}\.wiki.git/) { raise Gitlab::Shell::Error }
+
+            subject.execute
+          end
 
-          registry = Geo::ProjectRegistry.find_by(project_id: project.id)
+          it 'sets last_wiki_synced_at' do
+            expect(registry.last_wiki_synced_at).not_to be_nil
+          end
 
-          expect(registry.last_wiki_successful_sync_at).to be_nil
+          it 'resets last_wiki_successful_sync_at' do
+            expect(registry.last_wiki_successful_sync_at).to be_nil
+          end
         end
       end
     end
@@ -122,7 +154,7 @@ describe Geo::RepositorySyncService, services: true do
           expect { subject.execute }.not_to change(Geo::ProjectRegistry, :count)
         end
 
-        it 'does not update last_repository_successful_sync_at' do
+        it 'does not update last repository sync times' do
           subject.execute
 
           registry.reload
@@ -131,7 +163,7 @@ describe Geo::RepositorySyncService, services: true do
           expect(registry.last_repository_successful_sync_at).to be_within(1.minute).of(last_repository_synced_at)
         end
 
-        it 'does not update last_wiki_successful_sync_at' do
+        it 'does not update last wiki sync times' do
           subject.execute
 
           registry.reload
@@ -159,19 +191,24 @@ describe Geo::RepositorySyncService, services: true do
       end
 
       context 'tracking database' do
-        it 'sets last_repository_successful_sync_at' do
+        before do
           subject.execute
-
           registry.reload
+        end
 
-          expect(registry.last_repository_successful_sync_at).not_to be_nil
+        it 'updates last_repository_synced_at' do
+          expect(registry.last_repository_synced_at).to be_within(1.minute).of(DateTime.now)
         end
 
-        it 'sets last_wiki_successful_sync_at' do
-          subject.execute
+        it 'sets last_repository_successful_sync_at' do
+          expect(registry.last_repository_successful_sync_at).not_to be_nil
+        end
 
-          registry.reload
+        it 'updates last_wiki_synced_at' do
+          expect(registry.last_wiki_synced_at).to be_within(1.minute).of(DateTime.now)
+        end
 
+        it 'sets last_wiki_successful_sync_at' do
           expect(registry.last_wiki_successful_sync_at).not_to be_nil
         end
       end
@@ -211,29 +248,22 @@ describe Geo::RepositorySyncService, services: true do
       end
 
       context 'tracking database' do
-        it 'updates last_repository_successful_sync_at' do
+        before do
           subject.execute
-
           registry.reload
+        end
 
+        it 'updates last repository sync times' do
           expect(registry.last_repository_synced_at).to be_within(1.minute).of(DateTime.now)
           expect(registry.last_repository_successful_sync_at).to be_within(1.minute).of(DateTime.now)
         end
 
-        it 'does not update last_wiki_successful_sync_at' do
-          subject.execute
-
-          registry.reload
-
+        it 'does not update last wiki sync times' do
           expect(registry.last_wiki_synced_at).to be_within(1.minute).of(last_wiki_synced_at)
           expect(registry.last_wiki_successful_sync_at).to be_within(1.minute).of(last_wiki_synced_at)
         end
 
         it 'resets resync_repository' do
-          subject.execute
-
-          registry.reload
-
           expect(registry.resync_repository).to be false
         end
       end
@@ -271,29 +301,22 @@ describe Geo::RepositorySyncService, services: true do
       end
 
       context 'tracking database' do
-        it 'updates last_wiki_successful_sync_at' do
+        before do
           subject.execute
-
           registry.reload
+        end
 
+        it 'updates last wiki sync times' do
           expect(registry.last_wiki_synced_at).to be_within(1.minute).of(DateTime.now)
           expect(registry.last_wiki_successful_sync_at).to be_within(1.minute).of(DateTime.now)
         end
 
-        it 'does not update last_repository_successful_sync_at' do
-          subject.execute
-
-          registry.reload
-
+        it 'does not update last repository sync times' do
           expect(registry.last_repository_synced_at).to be_within(1.minute).of(last_repository_synced_at)
           expect(registry.last_repository_successful_sync_at).to be_within(1.minute).of(last_repository_synced_at)
         end
 
         it 'resets resync_wiki' do
-          subject.execute
-
-          registry.reload
-
           expect(registry.resync_wiki).to be false
         end
       end