Merge branch '238605-optimise-lfs-cleanup' into 'master'

Optimise cleaning up LFS objects

See merge request gitlab-org/gitlab!42830

app/workers/git_garbage_collect_worker.rb

@@ -95,6 +95,9 @@ class GitGarbageCollectWorker # rubocop:disable Scalability/IdempotentWorker
     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!
+  rescue => err
+    Gitlab::GitLogger.warn(message: "Cleaning up orphan LFS objects files failed", error: err.message)
+    Gitlab::ErrorTracking.track_and_raise_for_dev_exception(err)
   end
 
   def flush_ref_caches(project)

changelogs/unreleased/238605-optimise-lfs-cleanup.yml

+---
+title: Optimise cleaning up LFS objects
+merge_request: 42830
+author:
+type: performance

lib/gitlab/cleanup/orphan_lfs_file_references.rb

@@ -19,6 +19,11 @@ module Gitlab
       def run!
         log_info("Looking for orphan LFS files for project #{project.name_with_namespace}")
 
+        if project.lfs_objects.empty?
+          log_info("Project #{project.name_with_namespace} is linked to 0 LFS objects. Nothing to do")
+          return
+        end
+
         remove_orphan_references
       end
 

spec/lib/gitlab/cleanup/orphan_lfs_file_references_spec.rb

@@ -42,12 +42,24 @@ RSpec.describe Gitlab::Cleanup::OrphanLfsFileReferences do
       expect(null_logger).to receive(:info).with("Looking for orphan LFS files for project #{project.name_with_namespace}")
       expect(null_logger).to receive(:info).with("Removed invalid references: 1")
       expect(ProjectCacheWorker).to receive(:perform_async).with(project.id, [], [:lfs_objects_size])
+      expect(service).to receive(:remove_orphan_references).and_call_original
 
       expect { service.run! }.to change { project.lfs_objects.count }.from(2).to(1)
 
       expect(LfsObjectsProject.exists?(invalid_reference.id)).to be_falsey
     end
 
+    it 'does nothing if the project has no LFS objects' do
+      expect(null_logger).to receive(:info).with(/Looking for orphan LFS files/)
+      expect(null_logger).to receive(:info).with(/Nothing to do/)
+
+      project.lfs_objects_projects.delete_all
+
+      expect(service).not_to receive(:remove_orphan_references)
+
+      service.run!
+    end
+
     context 'LFS object is in design repository' do
       before do
         expect(project.design_repository).to receive(:exists?).and_return(true)

spec/workers/git_garbage_collect_worker_spec.rb

@@ -155,6 +155,17 @@ RSpec.describe GitGarbageCollectWorker do
 
               expect(project.lfs_objects.reload).to include(lfs_object)
             end
+
+            it 'catches and logs exceptions' do
+              expect_any_instance_of(Gitlab::Cleanup::OrphanLfsFileReferences)
+                .to receive(:run!)
+                .and_raise(/Failed/)
+
+              expect(Gitlab::GitLogger).to receive(:warn)
+              expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception)
+
+              subject.perform(*params)
+            end
           end
 
           context 'with cleanup_lfs_during_gc feature flag disabled' do