Refactoring Geo::RepositoryBackfillService

app/models/ee/repository.rb

+module EE
+  # Repository EE mixin
+  #
+  # This module is intended to encapsulate EE-specific model logic
+  # and be prepended in the `Repository` model
+  module Repository
+    extend ActiveSupport::Concern
+
+    # Runs code after a repository has been synced.
+    def after_sync
+      expire_all_method_caches
+      expire_branch_cache
+      expire_content_cache
+    end
+  end
+end

app/models/repository.rb

@@ -6,6 +6,7 @@ class Repository
   include Gitlab::ShellAdapter
   include Elastic::RepositoriesSearch
   include RepositoryMirroring
+  prepend EE::Repository
 
   attr_accessor :path_with_namespace, :project
 
@@ -426,13 +427,6 @@ class Repository
     expire_branches_cache
   end
 
-  # Runs code after a repository has been synced.
-  def after_sync
-    expire_all_method_caches
-    expire_branch_cache
-    expire_content_cache
-  end
-
   # Runs code after a new commit has been pushed.
   def after_push_commit(branch_name)
     expire_statistics_caches

app/services/geo/repository_backfill_service.rb

@@ -13,16 +13,14 @@ module Geo
     def execute
       try_obtain_lease do
         log('Started repository sync')
-
-        fetch_repositories do |started_at, finished_at|
-          update_registry(started_at, finished_at)
-        end
-
+        started_at, finished_at = fetch_repositories
+        update_registry(started_at, finished_at)
         log('Finished repository sync')
       end
     rescue ActiveRecord::RecordNotFound
       logger.error("Couldn't find project with ID=#{project_id}, skipping syncing")
     ensure
+      log('Releasing leases to sync repositories')
       Gitlab::ExclusiveLease.cancel(LEASE_KEY_PREFIX, backfill_lease)
     end
 
@@ -46,7 +44,7 @@ module Geo
         Rails.logger.error "Error syncing repository for project #{project.path_with_namespace}: #{e}"
       end
 
-      yield started_at, finished_at
+      [started_at, finished_at]
     end
 
     def fetch_project_repository
@@ -80,6 +78,9 @@ module Geo
 
       yield
 
+      # We should release the lease for a repository, only if we have obtained
+      # it. If something went wrong when syncing the repository, we should wait
+      # for the lease timeout to try again.
       log('Releasing leases to sync repository')
       Gitlab::ExclusiveLease.cancel(lease_key, repository_lease)
     end
@@ -93,7 +94,7 @@ module Geo
     end
 
     def lease_key
-      @key ||= "#{LEASE_KEY_PREFIX}:#{project.id}"
+      @lease_key ||= "#{LEASE_KEY_PREFIX}:#{project.id}"
     end
 
     def primary_ssh_path_prefix

spec/models/repository_spec.rb

@@ -1972,16 +1972,6 @@ describe Repository, models: true do
     end
   end
 
-  describe '#after_sync' do
-    it 'expires repository cache' do
-      expect(repository).to receive(:expire_all_method_caches)
-      expect(repository).to receive(:expire_branch_cache)
-      expect(repository).to receive(:expire_content_cache)
-
-      repository.after_sync
-    end
-  end
-
   def create_remote_branch(remote_name, branch_name, target)
     rugged = repository.rugged
     rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id)

spec/services/geo/repository_backfill_service_spec.rb

@@ -22,7 +22,9 @@ describe Geo::RepositoryBackfillService, services: true do
     it 'expires repository caches' do
       allow_any_instance_of(Repository).to receive(:fetch_geo_mirror) { true }
 
-      expect_any_instance_of(Repository).to receive(:after_sync).once
+      expect_any_instance_of(Repository).to receive(:expire_all_method_caches).once
+      expect_any_instance_of(Repository).to receive(:expire_branch_cache).once
+      expect_any_instance_of(Repository).to receive(:expire_content_cache).once
 
       subject.execute
     end