Merge branch '5181-sync-repack' into 'master'

"Geo: schedule a git repack after initial clone"

Closes #5181

See merge request gitlab-org/gitlab-ee!5261

ee/app/services/geo/base_sync_service.rb

@@ -61,10 +61,18 @@ module Geo
       if redownload
         redownload_repository
         set_temp_repository_as_main
+        schedule_repack
+      elsif repository.exists?
+        fetch_geo_mirror(repository)
       else
         ensure_repository
         fetch_geo_mirror(repository)
+        schedule_repack
+      end
     end
+
+    def schedule_repack
+      raise NotImplementedError
     end
 
     def redownload_repository

ee/app/services/geo/repository_sync_service.rb

@@ -62,5 +62,9 @@ module Geo
 
       repository.copy_gitattributes(project.default_branch)
     end
+
+    def schedule_repack
+      GitGarbageCollectWorker.perform_async(@project.id, :full_repack, lease_key)
+    end
   end
 end

ee/app/services/geo/wiki_sync_service.rb

@@ -51,5 +51,10 @@ module Geo
                update_delay_s: update_delay_in_seconds,
                download_time_s: download_time_in_seconds)
     end
+
+    def schedule_repack
+      # No-op: we currently don't schedule wiki repository to repack
+      # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/45523
+    end
   end
 end

ee/changelogs/unreleased/5181-sync-repack.yml

+---
+title: 'Geo: schedule a git repack after initial clone'
+merge_request: 4266
+author:
+type: added

ee/spec/services/geo/repository_sync_service_spec.rb

@@ -7,6 +7,8 @@ describe Geo::RepositorySyncService do
   set(:secondary) { create(:geo_node) }
 
   let(:lease) { double(try_obtain: true) }
+  let(:project) { create(:project_empty_repo) }
+  let(:repository) { project.repository }
 
   subject { described_class.new(project) }
 
@@ -15,16 +17,13 @@ describe Geo::RepositorySyncService do
   end
 
   it_behaves_like 'geo base sync execution'
+  it_behaves_like 'geo base sync fetch and repack'
 
   describe '#execute' do
-    let(:project) { create(:project_empty_repo) }
-    let(:repository) { project.repository }
     let(:url_to_repo) { "#{primary.url}#{project.full_path}.git" }
 
     before do
-      allow(Gitlab::ExclusiveLease).to receive(:new)
-        .with(subject.lease_key, anything)
-        .and_return(lease)
+      allow(subject).to receive(:exclusive_lease).and_return(lease)
 
       allow_any_instance_of(Repository).to receive(:fetch_as_mirror)
         .and_return(true)
@@ -340,4 +339,12 @@ describe Geo::RepositorySyncService do
       let(:repository) { project.repository }
     end
   end
+
+  describe '#schedule_repack' do
+    it 'schedule GitGarbageCollectWorker for full repack' do
+      Sidekiq::Testing.fake! do
+        expect { subject.send(:schedule_repack) }.to change { GitGarbageCollectWorker.jobs.count }.by(1)
+      end
+    end
+  end
 end

ee/spec/services/geo/wiki_sync_service_spec.rb

@@ -6,7 +6,9 @@ RSpec.describe Geo::WikiSyncService do
   set(:primary) { create(:geo_node, :primary) }
   set(:secondary) { create(:geo_node) }
 
-  let(:lease) { double(try_obtain: true) }
+  let(:lease) { double(try_obtain: true, lease_key: anything ) }
+  let(:project) { create(:project_empty_repo) }
+  let(:repository) { project.wiki.repository }
 
   subject { described_class.new(project) }
 
@@ -15,16 +17,13 @@ RSpec.describe Geo::WikiSyncService do
   end
 
   it_behaves_like 'geo base sync execution'
+  it_behaves_like 'geo base sync fetch and repack'
 
   describe '#execute' do
-    let(:project) { create(:project_empty_repo) }
-    let(:repository) { project.wiki.repository }
     let(:url_to_repo) { "#{primary.url}#{project.full_path}.wiki.git" }
 
     before do
-      allow(Gitlab::ExclusiveLease).to receive(:new)
-        .with(subject.lease_key, anything)
-        .and_return(lease)
+      allow(subject).to receive(:exclusive_lease).and_return(lease)
 
       allow_any_instance_of(Repository).to receive(:fetch_as_mirror)
         .and_return(true)
@@ -40,7 +39,7 @@ RSpec.describe Geo::WikiSyncService do
     end
 
     it 'releases lease' do
-      expect(Gitlab::ExclusiveLease).to receive(:cancel).once.with(
+      expect(Gitlab::ExclusiveLease).to receive(:cancel).at_least(:once).with(
         subject.__send__(:lease_key), anything).and_call_original
 
       subject.execute
@@ -49,7 +48,7 @@ RSpec.describe Geo::WikiSyncService do
     it 'voids the failure message when it succeeds after an error' do
       registry = create(:geo_project_registry, project: project, last_wiki_sync_failure: 'error')
 
-      expect { subject.execute }.to change { registry.reload.last_wiki_sync_failure}.to(nil)
+      expect { subject.execute }.to change { registry.reload.last_wiki_sync_failure }.to(nil)
     end
 
     it 'does not fetch wiki repository if cannot obtain a lease' do

ee/spec/support/services/base_sync_service_shared_examples.rb

@@ -48,6 +48,76 @@ shared_examples 'cleans temporary repositories' do
   end
 end
 
+shared_examples 'geo base sync fetch and repack' do
+  describe '#fetch_repository' do
+    let(:fetch_repository) { subject.send(:fetch_repository, false) }
+
+    before do
+      allow(subject).to receive(:fetch_geo_mirror).and_return(true)
+    end
+
+    it 'cleans up temporary repository' do
+      is_expected.to receive(:clean_up_temporary_repository)
+
+      fetch_repository
+    end
+
+    it 'updates registry' do
+      is_expected.to receive(:update_registry!)
+
+      fetch_repository
+    end
+
+    it 'fetches repository from geo node' do
+      is_expected.to receive(:fetch_geo_mirror).with(subject.send(:repository))
+
+      fetch_repository
+    end
+
+    context 'redownload' do
+      let(:fetch_repository) { subject.send(:fetch_repository, true) }
+
+      before do
+        stub_feature_flags(geo_redownload_with_snapshot: false)
+      end
+
+      it 'fetches repository from geo node into temporary location' do
+        temporary_repository = instance_double('Repository')
+
+        is_expected.to receive(:temp_repo) { temporary_repository }
+        is_expected.to receive(:fetch_geo_mirror).with(temporary_repository)
+        is_expected.to receive(:set_temp_repository_as_main)
+
+        fetch_repository
+      end
+
+      it 'schedule git repack' do
+        is_expected.to receive(:schedule_repack)
+
+        fetch_repository
+      end
+    end
+
+    context 'repository does not exist' do
+      before do
+        allow_any_instance_of(Repository).to receive(:exists?) { false }
+      end
+
+      it 'ensures repository is created' do
+        is_expected.to receive(:ensure_repository)
+
+        fetch_repository
+      end
+
+      it 'schedule git repack' do
+        is_expected.to receive(:schedule_repack)
+
+        fetch_repository
+      end
+    end
+  end
+end
+
 shared_examples 'sync retries use the snapshot RPC' do
   let(:retry_count) { Geo::BaseSyncService::RETRY_BEFORE_REDOWNLOAD }