Merge branch '223621-prune-lfs-objects-during-git-gc' into 'master'

Clean up orphaned LFS file references during GC

Closes #223621

See merge request gitlab-org/gitlab!38813

app/workers/git_garbage_collect_worker.rb

@@ -27,7 +27,10 @@ class GitGarbageCollectWorker # rubocop:disable Scalability/IdempotentWorker
 
     task = task.to_sym
 
-    ::Projects::GitDeduplicationService.new(project).execute if task == :gc
+    if task == :gc
+      ::Projects::GitDeduplicationService.new(project).execute
+      cleanup_orphan_lfs_file_references(project)
+    end
 
     gitaly_call(task, project.repository.raw_repository)
 
@@ -90,6 +93,13 @@ class GitGarbageCollectWorker # rubocop:disable Scalability/IdempotentWorker
     raise Gitlab::Git::CommandError.new(e)
   end
 
+  def cleanup_orphan_lfs_file_references(project)
+    return unless Feature.enabled?(:cleanup_lfs_during_gc, project)
+    return if Gitlab::Database.read_only? # GitGarbageCollectWorker may be run on a Geo secondary
+
+    ::Gitlab::Cleanup::OrphanLfsFileReferences.new(project, dry_run: false, logger: logger).run!
+  end
+
   def flush_ref_caches(project)
     project.repository.after_create_branch
     project.repository.branch_names

changelogs/unreleased/223621-prune-lfs-objects-during-git-gc.yml

+---
+title: Clean up orphaned LFS file references during GC
+merge_request: 38813
+author:
+type: added

spec/workers/git_garbage_collect_worker_spec.rb

@@ -7,7 +7,7 @@ require 'spec_helper'
 RSpec.describe GitGarbageCollectWorker do
   include GitHelpers
 
-  let(:project) { create(:project, :repository) }
+  let_it_be(:project) { create(:project, :repository) }
   let(:shell) { Gitlab::Shell.new }
   let!(:lease_uuid) { SecureRandom.uuid }
   let!(:lease_key) { "project_housekeeping:#{project.id}" }
@@ -114,6 +114,56 @@ RSpec.describe GitGarbageCollectWorker do
             subject.perform(*params)
           end
         end
+
+        context 'LFS object garbage collection' do
+          before do
+            stub_lfs_setting(enabled: true)
+          end
+
+          let_it_be(:lfs_reference) { create(:lfs_objects_project, project: project) }
+          let(:lfs_object) { lfs_reference.lfs_object }
+
+          context 'with cleanup_lfs_during_gc feature flag enabled' do
+            before do
+              stub_feature_flags(cleanup_lfs_during_gc: true)
+            end
+
+            it 'cleans up unreferenced LFS objects' do
+              expect_next_instance_of(Gitlab::Cleanup::OrphanLfsFileReferences) do |svc|
+                expect(svc.project).to eq(project)
+                expect(svc.dry_run).to be_falsy
+                expect(svc).to receive(:run!).and_call_original
+              end
+
+              subject.perform(*params)
+
+              expect(project.lfs_objects.reload).not_to include(lfs_object)
+            end
+
+            it 'does nothing if the database is read-only' do
+              expect(Gitlab::Database).to receive(:read_only?) { true }
+              expect_any_instance_of(Gitlab::Cleanup::OrphanLfsFileReferences).not_to receive(:run!)
+
+              subject.perform(*params)
+
+              expect(project.lfs_objects.reload).to include(lfs_object)
+            end
+          end
+
+          context 'with cleanup_lfs_during_gc feature flag disabled' do
+            before do
+              stub_feature_flags(cleanup_lfs_during_gc: false)
+            end
+
+            it 'does not clean up unreferenced LFS objects' do
+              expect_any_instance_of(Gitlab::Cleanup::OrphanLfsFileReferences).not_to receive(:run!)
+
+              subject.perform(*params)
+
+              expect(project.lfs_objects.reload).to include(lfs_object)
+            end
+          end
+        end
       end
 
       context 'when no lease can be obtained' do