Commit f6eb1edc authored by Stan Hu's avatar Stan Hu

Skip LFS updates in mirrors if repository has not changed

The GetAllLFSPointers RPC is expensive and was being called for every
mirror. If the repository has not changed, we can assume that the LFS
pointers also did not change. We can skip calling this RPC if there are
no updates.

Since we don't actually get a return code if branches or tags were
updated, we use the repository checksum as a cheaper mechanism (which
XORs the ref contents) to determine whether we run the LFS scan.

Closes https://gitlab.com/gitlab-org/gitlab/issues/38256
parent 262bcdd6
---
title: Skip updating LFS objects in mirror updates if repository has not changed
merge_request: 21744
author:
type: performance
......@@ -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
......
......@@ -325,13 +325,26 @@ describe Projects::UpdateMirrorService do
end
end
context 'updating Lfs objects' do
context 'updating LFS objects' do
context 'when repository does not change' do
before do
allow(project).to receive(:lfs_enabled?).and_return(true)
end
it 'does not attempt to update LFS objects' do
expect(Projects::LfsPointers::LfsImportService).not_to receive(:new)
service.execute
end
end
context 'when repository changes' do
before do
stub_fetch_mirror(project)
end
context 'when Lfs is disabled in the project' do
it 'does not update Lfs objects' 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)
......@@ -344,7 +357,7 @@ describe Projects::UpdateMirrorService do
allow(project).to receive(:lfs_enabled?).and_return(true)
end
it 'updates Lfs objects' do
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({})
......@@ -383,6 +396,7 @@ describe Projects::UpdateMirrorService do
end
end
end
end
it "fails when the mirror user doesn't have access" do
stub_fetch_mirror(project)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment