Merge branch 'gitaly-2108-repos-gc-after-move' into 'master'

Run housekeeping after moving a repository between shards

See merge request gitlab-org/gitlab!20863

changelogs/unreleased/gitaly-2108-repos-gc-after-move.yml

+---
+title: Run housekeeping after moving a repository between shards
+merge_request: 20863
+author:
+type: performance

ee/app/services/projects/update_repository_storage_service.rb

@@ -33,6 +33,8 @@ module Projects
         project.update(repository_storage: new_repository_storage_key, repository_read_only: false)
         project.leave_pool_repository
         project.track_project_repository
+
+        enqueue_housekeeping
       else
         project.update(repository_read_only: false)
       end
@@ -89,6 +91,18 @@ module Projects
       "#{path}+#{project.id}+moved+#{Time.now.to_i}"
     end
 
+    # The underlying FetchInternalRemote call uses a `git fetch` to move data
+    # to the new repository, which leaves it in a less-well-packed state and
+    # lacking bitmaps. Housekeeping will boost performance significantly.
+    def enqueue_housekeeping
+      return unless Gitlab::CurrentSettings.housekeeping_enabled?
+      return unless Feature.enabled?(:repack_after_shard_migration, project)
+
+      Projects::HousekeepingService.new(project, :full_repack).execute
+    rescue Projects::HousekeepingService::LeaseTaken
+      # No action required
+    end
+
     def wait_for_pushes(type)
       reference_counter = project.reference_counter(type: type)
 

ee/spec/services/projects/update_repository_storage_service_spec.rb

@@ -73,7 +73,19 @@ describe Projects::UpdateRepositoryStorageService do
           .and_return(repository_double)
       end
 
-      context 'when the move succeeds' do
+      context 'when the move succeeds', :clean_gitlab_redis_shared_state do
+        before do
+          allow(project_repository_double)
+            .to receive(:fetch_repository_as_mirror)
+            .with(project.repository.raw)
+            .and_return(true)
+
+          allow(repository_double)
+            .to receive(:fetch_repository_as_mirror)
+            .with(repository.raw)
+            .and_return(true)
+        end
+
         it "moves the project and its #{repository_type} repository to the new storage and unmarks the repository as read only" do
           old_project_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
             project.repository.path_to_repo
@@ -81,12 +93,6 @@ describe Projects::UpdateRepositoryStorageService do
 
           old_repository_path = repository.full_path
 
-          allow(project_repository_double).to receive(:fetch_repository_as_mirror)
-            .with(project.repository.raw).and_return(true)
-
-          allow(repository_double).to receive(:fetch_repository_as_mirror)
-            .with(repository.raw).and_return(true)
-
           subject.execute('test_second_storage')
 
           expect(project).not_to be_repository_read_only
@@ -94,6 +100,35 @@ describe Projects::UpdateRepositoryStorageService do
           expect(gitlab_shell.repository_exists?('default', old_project_repository_path)).to be(false)
           expect(gitlab_shell.repository_exists?('default', old_repository_path)).to be(false)
         end
+
+        context ':repack_after_shard_migration feature flag disabled' do
+          before do
+            stub_feature_flags(repack_after_shard_migration: false)
+          end
+
+          it 'does not enqueue a GC run' do
+            expect { subject.execute('test_second_storage') }
+              .not_to change(GitGarbageCollectWorker.jobs, :count)
+          end
+        end
+
+        context ':repack_after_shard_migration feature flag enabled' do
+          before do
+            stub_feature_flags(repack_after_shard_migration: true)
+          end
+
+          it 'does not enqueue a GC run if housekeeping is disabled' do
+            stub_application_setting(housekeeping_enabled: false)
+
+            expect { subject.execute('test_second_storage') }
+              .not_to change(GitGarbageCollectWorker.jobs, :count)
+          end
+
+          it 'enqueues a GC run' do
+            expect { subject.execute('test_second_storage') }
+              .to change(GitGarbageCollectWorker.jobs, :count).by(1)
+          end
+        end
       end
 
       context 'when the project is already on the target storage' do