Prune loose objects when running git gc during repository cleanup

Without this, housekeeping may not make some objects unavailable
immediately, and disk space may go up instead of down. We've marked
the repository as read-only for the duration of the operation, so it
is safe to do this.

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