Merge branch 'sh-skip-lfs-updates-mirroring' into 'master'

Skip updating LFS objects in mirror updates if repository has not changed

Closes #38256

See merge request gitlab-org/gitlab!21744

changelogs/unreleased/sh-skip-lfs-updates-mirroring.yml

+---
+title: Skip updating LFS objects in mirror updates if repository has not changed
+merge_request: 21744
+author:
+type: performance

ee/app/services/projects/update_mirror_service.rb

@@ -24,12 +24,17 @@ module Projects
         return error("The mirror user is not allowed to push code to all branches on this project.")
       end
 
+      checksum_before = project.repository.checksum
+
       update_tags do
         project.fetch_mirror(forced: true)
       end
 
       update_branches
-      update_lfs_objects
+
+      # Updating LFS objects is expensive since it requires scanning for blobs with pointers.
+      # Let's skip this if the repository hasn't changed.
+      update_lfs_objects if project.repository.checksum != checksum_before
 
       success
     rescue Gitlab::Shell::Error, Gitlab::Git::BaseError, UpdateError => e

ee/spec/services/projects/update_mirror_service_spec.rb

@@ -325,60 +325,74 @@ describe Projects::UpdateMirrorService do
       end
     end
 
-    context 'updating Lfs objects' do
-      before do
-        stub_fetch_mirror(project)
-      end
+    context 'updating LFS objects' do
+      context 'when repository does not change' do
+        before do
+          allow(project).to receive(:lfs_enabled?).and_return(true)
+        end
 
-      context 'when Lfs is disabled in the project' do
-        it 'does not update Lfs objects' do
-          allow(project).to receive(:lfs_enabled?).and_return(false)
-          expect(Projects::LfsPointers::LfsObjectDownloadListService).not_to receive(:new)
+        it 'does not attempt to update LFS objects' do
+          expect(Projects::LfsPointers::LfsImportService).not_to receive(:new)
 
           service.execute
         end
       end
 
-      context 'when Lfs is enabled in the project' do
+      context 'when repository changes' do
         before do
-          allow(project).to receive(:lfs_enabled?).and_return(true)
+          stub_fetch_mirror(project)
         end
 
-        it 'updates Lfs objects' do
-          expect(Projects::LfsPointers::LfsImportService).to receive(:new).and_call_original
-          expect_any_instance_of(Projects::LfsPointers::LfsObjectDownloadListService).to receive(:execute).and_return({})
+        context 'when Lfs is disabled in the project' do
+          it 'does not update LFS objects' do
+            allow(project).to receive(:lfs_enabled?).and_return(false)
+            expect(Projects::LfsPointers::LfsObjectDownloadListService).not_to receive(:new)
 
-          service.execute
+            service.execute
+          end
         end
 
-        context 'when Lfs import fails' do
-          let(:error_message) { 'error_message' }
-
+        context 'when Lfs is enabled in the project' do
           before do
-            expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: error_message)
+            allow(project).to receive(:lfs_enabled?).and_return(true)
           end
 
-          # Uncomment once https://gitlab.com/gitlab-org/gitlab-foss/issues/61834 is closed
-          # it 'fails mirror operation' do
-          #   expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: 'error message')
+          it 'updates LFS objects' do
+            expect(Projects::LfsPointers::LfsImportService).to receive(:new).and_call_original
+            expect_any_instance_of(Projects::LfsPointers::LfsObjectDownloadListService).to receive(:execute).and_return({})
 
-          #   result = subject.execute
+            service.execute
+          end
+
+          context 'when Lfs import fails' do
+            let(:error_message) { 'error_message' }
 
-          #   expect(result[:status]).to eq :error
-          #   expect(result[:message]).to eq 'error message'
-          # end
+            before do
+              expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: error_message)
+            end
 
-          # Remove once https://gitlab.com/gitlab-org/gitlab-foss/issues/61834 is closed
-          it 'does not fail mirror operation' do
-            result = subject.execute
+            # Uncomment once https://gitlab.com/gitlab-org/gitlab-foss/issues/61834 is closed
+            # it 'fails mirror operation' do
+            #   expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: 'error message')
 
-            expect(result[:status]).to eq :success
-          end
+            #   result = subject.execute
 
-          it 'logs the error' do
-            expect_any_instance_of(Gitlab::UpdateMirrorServiceJsonLogger).to receive(:error).with(hash_including(error_message: error_message))
+            #   expect(result[:status]).to eq :error
+            #   expect(result[:message]).to eq 'error message'
+            # end
 
-            subject.execute
+            # Remove once https://gitlab.com/gitlab-org/gitlab-foss/issues/61834 is closed
+            it 'does not fail mirror operation' do
+              result = subject.execute
+
+              expect(result[:status]).to eq :success
+            end
+
+            it 'logs the error' do
+              expect_any_instance_of(Gitlab::UpdateMirrorServiceJsonLogger).to receive(:error).with(hash_including(error_message: error_message))
+
+              subject.execute
+            end
           end
         end
       end