Merge branch '220104-git-gc-should-prune-objects' into 'master'

Improve running git gc during repository cleanup

See merge request gitlab-org/gitlab!39592

app/services/projects/cleanup_service.rb

@@ -40,7 +40,7 @@ module Projects
       apply_bfg_object_map!
 
       # Remove older objects that are no longer referenced
-      GitGarbageCollectWorker.new.perform(project.id, :gc, "project_cleanup:gc:#{project.id}")
+      GitGarbageCollectWorker.new.perform(project.id, :prune, "project_cleanup:gc:#{project.id}")
 
       # The cache may now be inaccurate, and holding onto it could prevent
       # bugs assuming the presence of some object from manifesting for some

app/workers/git_garbage_collect_worker.rb

@@ -27,15 +27,15 @@ class GitGarbageCollectWorker # rubocop:disable Scalability/IdempotentWorker
 
     task = task.to_sym
 
-    if task == :gc
+    if gc?(task)
       ::Projects::GitDeduplicationService.new(project).execute
       cleanup_orphan_lfs_file_references(project)
     end
 
-    gitaly_call(task, project.repository.raw_repository)
+    gitaly_call(task, project)
 
     # Refresh the branch cache in case garbage collection caused a ref lookup to fail
-    flush_ref_caches(project) if task == :gc
+    flush_ref_caches(project) if gc?(task)
 
     update_repository_statistics(project) if task != :pack_refs
 
@@ -48,6 +48,10 @@ class GitGarbageCollectWorker # rubocop:disable Scalability/IdempotentWorker
 
   private
 
+  def gc?(task)
+    task == :gc || task == :prune
+  end
+
   def try_obtain_lease(key)
     ::Gitlab::ExclusiveLease.new(key, timeout: LEASE_TIMEOUT).try_obtain
   end
@@ -64,8 +68,9 @@ class GitGarbageCollectWorker # rubocop:disable Scalability/IdempotentWorker
     ::Gitlab::ExclusiveLease.get_uuid(key)
   end
 
-  ## `repository` has to be a Gitlab::Git::Repository
-  def gitaly_call(task, repository)
+  def gitaly_call(task, project)
+    repository = project.repository.raw_repository
+
     client = if task == :pack_refs
                Gitlab::GitalyClient::RefService.new(repository)
              else
@@ -73,8 +78,8 @@ class GitGarbageCollectWorker # rubocop:disable Scalability/IdempotentWorker
              end
 
     case task
-    when :gc
-      client.garbage_collect(bitmaps_enabled?)
+    when :prune, :gc
+      client.garbage_collect(bitmaps_enabled?, prune: task == :prune)
     when :full_repack
       client.repack_full(bitmaps_enabled?)
     when :incremental_repack

changelogs/unreleased/220104-git-gc-should-prune-objects.yml

+---
+title: Prune loose objects during git garbage collection
+merge_request: 39592
+author:
+type: changed

lib/gitlab/gitaly_client/repository_service.rb

@@ -26,8 +26,8 @@ module Gitlab
         GitalyClient.call(@storage, :repository_service, :cleanup, request, timeout: GitalyClient.fast_timeout)
       end
 
-      def garbage_collect(create_bitmap)
-        request = Gitaly::GarbageCollectRequest.new(repository: @gitaly_repo, create_bitmap: create_bitmap)
+      def garbage_collect(create_bitmap, prune:)
+        request = Gitaly::GarbageCollectRequest.new(repository: @gitaly_repo, create_bitmap: create_bitmap, prune: prune)
         GitalyClient.call(@storage, :repository_service, :garbage_collect, request, timeout: GitalyClient.long_timeout)
       end
 

spec/lib/gitlab/gitaly_client/repository_service_spec.rb

@@ -38,7 +38,7 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do
         .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
         .and_return(double(:garbage_collect_response))
 
-      client.garbage_collect(true)
+      client.garbage_collect(true, prune: true)
     end
   end
 

spec/services/projects/cleanup_service_spec.rb

@@ -89,7 +89,7 @@ RSpec.describe Projects::CleanupService do
 
     it 'runs garbage collection on the repository' do
       expect_next_instance_of(GitGarbageCollectWorker) do |worker|
-        expect(worker).to receive(:perform).with(project.id, :gc, "project_cleanup:gc:#{project.id}")
+        expect(worker).to receive(:perform).with(project.id, :prune, "project_cleanup:gc:#{project.id}")
       end
 
       service.execute

spec/workers/git_garbage_collect_worker_spec.rb

@@ -272,6 +272,11 @@ RSpec.describe GitGarbageCollectWorker do
 
         expect(before_packs.count).to be >= 1
 
+        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService)
+          .to receive(:garbage_collect)
+          .with(bitmaps_enabled, prune: false)
+          .and_call_original
+
         subject.perform(project.id, 'gc', lease_key, lease_uuid)
         after_packed_refs = packed_refs(project)
         after_packs = packs(project)
@@ -292,6 +297,15 @@ RSpec.describe GitGarbageCollectWorker do
 
         subject.perform(project.id, 'gc', lease_key, lease_uuid)
       end
+
+      it 'prune calls garbage_collect with the option prune: true' do
+        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService)
+          .to receive(:garbage_collect)
+          .with(bitmaps_enabled, prune: true)
+          .and_return(nil)
+
+        subject.perform(project.id, 'prune', lease_key, lease_uuid)
+      end
     end
 
     context 'with bitmaps enabled' do