Commit 5d0406e9 authored by Marc Shaw's avatar Marc Shaw

Immediately update repo stats after garbage collection

We were facing an issue where the stats reflected on the project page
were incorrect because the clean up wasn't being run straight away.

Issue: gitlab.com/gitlab-org/gitlab/-/issues/226927
Merge Request: gitlab.com/gitlab-org/gitlab/-/merge_requests/37579
parent 8e15ea3b
...@@ -33,7 +33,10 @@ class GitGarbageCollectWorker # rubocop:disable Scalability/IdempotentWorker ...@@ -33,7 +33,10 @@ class GitGarbageCollectWorker # rubocop:disable Scalability/IdempotentWorker
# Refresh the branch cache in case garbage collection caused a ref lookup to fail # 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 task == :gc
project.repository.expire_statistics_caches if task != :pack_refs if task != :pack_refs
project.repository.expire_statistics_caches
Projects::UpdateStatisticsService.new(project, nil, statistics: [:repository_size, :lfs_objects_size]).execute
end
# In case pack files are deleted, release libgit2 cache and open file # In case pack files are deleted, release libgit2 cache and open file
# descriptors ASAP instead of waiting for Ruby garbage collection # descriptors ASAP instead of waiting for Ruby garbage collection
......
---
title: Immediately update project statistics when running housekeeping or repository
cleanup
merge_request: 37579
author:
type: other
...@@ -11,31 +11,57 @@ RSpec.describe GitGarbageCollectWorker do ...@@ -11,31 +11,57 @@ RSpec.describe GitGarbageCollectWorker do
let(:shell) { Gitlab::Shell.new } let(:shell) { Gitlab::Shell.new }
let!(:lease_uuid) { SecureRandom.uuid } let!(:lease_uuid) { SecureRandom.uuid }
let!(:lease_key) { "project_housekeeping:#{project.id}" } let!(:lease_key) { "project_housekeeping:#{project.id}" }
let(:params) { [project.id, task, lease_key, lease_uuid] }
subject { described_class.new } subject { described_class.new }
shared_examples 'it calls Gitaly' do
specify do
expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(gitaly_task)
.and_return(nil)
subject.perform(*params)
end
end
shared_examples 'it updates the project statistics' do
specify do
expect_any_instance_of(Projects::UpdateStatisticsService).to receive(:execute).and_call_original
expect(Projects::UpdateStatisticsService)
.to receive(:new)
.with(project, nil, statistics: [:repository_size, :lfs_objects_size])
.and_call_original
subject.perform(*params)
end
end
describe "#perform" do describe "#perform" do
let(:gitaly_task) { :garbage_collect }
let(:task) { :gc }
context 'with active lease_uuid' do context 'with active lease_uuid' do
before do before do
allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid) allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
end end
it_behaves_like 'it calls Gitaly'
it_behaves_like 'it updates the project statistics'
it "flushes ref caches when the task if 'gc'" do it "flushes ref caches when the task if 'gc'" do
expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original
expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect)
.and_return(nil)
expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
expect_any_instance_of(Repository).to receive(:branch_names).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original
expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
subject.perform(project.id, :gc, lease_key, lease_uuid) subject.perform(*params)
end end
it 'handles gRPC errors' do it 'handles gRPC errors' do
expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect).and_raise(GRPC::NotFound) expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect).and_raise(GRPC::NotFound)
expect { subject.perform(project.id, :gc, lease_key, lease_uuid) }.to raise_exception(Gitlab::Git::Repository::NoRepository) expect { subject.perform(*params) }.to raise_exception(Gitlab::Git::Repository::NoRepository)
end end
end end
...@@ -49,11 +75,13 @@ RSpec.describe GitGarbageCollectWorker do ...@@ -49,11 +75,13 @@ RSpec.describe GitGarbageCollectWorker do
expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
subject.perform(project.id, :gc, lease_key, lease_uuid) subject.perform(*params)
end end
end end
context 'with no active lease' do context 'with no active lease' do
let(:params) { [project.id] }
before do before do
allow(subject).to receive(:get_lease_uuid).and_return(false) allow(subject).to receive(:get_lease_uuid).and_return(false)
end end
...@@ -63,15 +91,16 @@ RSpec.describe GitGarbageCollectWorker do ...@@ -63,15 +91,16 @@ RSpec.describe GitGarbageCollectWorker do
allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid) allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid)
end end
it_behaves_like 'it calls Gitaly'
it_behaves_like 'it updates the project statistics'
it "flushes ref caches when the task if 'gc'" do it "flushes ref caches when the task if 'gc'" do
expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect)
.and_return(nil)
expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
expect_any_instance_of(Repository).to receive(:branch_names).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original
expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
subject.perform(project.id) subject.perform(*params)
end end
context 'when the repository has joined a pool' do context 'when the repository has joined a pool' do
...@@ -81,7 +110,7 @@ RSpec.describe GitGarbageCollectWorker do ...@@ -81,7 +110,7 @@ RSpec.describe GitGarbageCollectWorker do
it 'ensures the repositories are linked' do it 'ensures the repositories are linked' do
expect_any_instance_of(PoolRepository).to receive(:link_repository).once expect_any_instance_of(PoolRepository).to receive(:link_repository).once
subject.perform(project.id) subject.perform(*params)
end end
end end
end end
...@@ -97,48 +126,55 @@ RSpec.describe GitGarbageCollectWorker do ...@@ -97,48 +126,55 @@ RSpec.describe GitGarbageCollectWorker do
expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
subject.perform(project.id) subject.perform(*params)
end end
end end
end end
context "repack_full" do context "repack_full" do
let(:task) { :full_repack }
let(:gitaly_task) { :repack_full }
before do before do
expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid) expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
end end
it "calls Gitaly" do it_behaves_like 'it calls Gitaly'
expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:repack_full) it_behaves_like 'it updates the project statistics'
.and_return(nil)
subject.perform(project.id, :full_repack, lease_key, lease_uuid)
end
end end
context "pack_refs" do context "pack_refs" do
let(:task) { :pack_refs }
let(:gitaly_task) { :pack_refs }
before do before do
expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid) expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
end end
it "calls Gitaly" do it "calls Gitaly" do
expect_any_instance_of(Gitlab::GitalyClient::RefService).to receive(:pack_refs) expect_any_instance_of(Gitlab::GitalyClient::RefService).to receive(task)
.and_return(nil) .and_return(nil)
subject.perform(project.id, :pack_refs, lease_key, lease_uuid) subject.perform(*params)
end
it 'does not update the project statistics' do
expect(Projects::UpdateStatisticsService).not_to receive(:new)
subject.perform(*params)
end end
end end
context "repack_incremental" do context "repack_incremental" do
let(:task) { :incremental_repack }
let(:gitaly_task) { :repack_incremental }
before do before do
expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid) expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
end end
it "calls Gitaly" do it_behaves_like 'it calls Gitaly'
expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:repack_incremental) it_behaves_like 'it updates the project statistics'
.and_return(nil)
subject.perform(project.id, :incremental_repack, lease_key, lease_uuid)
end
end end
shared_examples 'gc tasks' do shared_examples 'gc tasks' do
......
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