Merge branch...

Merge branch '226927-immediately-update-repository-statistics-when-running-housekeeping-and-repository-cleanup' into 'master'

Resolve "Immediately update repository statistics when running housekeeping and repository cleanup"

See merge request gitlab-org/gitlab!37579

app/workers/git_garbage_collect_worker.rb

@@ -33,7 +33,10 @@ class GitGarbageCollectWorker # rubocop:disable Scalability/IdempotentWorker
     # Refresh the branch cache in case garbage collection caused a ref lookup to fail
     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
     # descriptors ASAP instead of waiting for Ruby garbage collection

changelogs/unreleased/226927-immediately-update-repository-statistics-when-running-housekeeping.yml

+---
+title: Immediately update project statistics when running housekeeping or repository
+  cleanup
+merge_request: 37579
+author:
+type: other

spec/workers/git_garbage_collect_worker_spec.rb

@@ -11,31 +11,57 @@ RSpec.describe GitGarbageCollectWorker do
   let(:shell) { Gitlab::Shell.new }
   let!(:lease_uuid) { SecureRandom.uuid }
   let!(:lease_key) { "project_housekeeping:#{project.id}" }
+  let(:params) { [project.id, task, lease_key, lease_uuid] }
 
   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
+    let(:gitaly_task) { :garbage_collect }
+    let(:task) { :gc }
+
     context 'with active lease_uuid' do
       before do
         allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
       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
         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(:branch_names).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
 
-        subject.perform(project.id, :gc, lease_key, lease_uuid)
+        subject.perform(*params)
       end
 
       it 'handles gRPC errors' do
         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
 
@@ -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(:has_visible_content?).and_call_original
 
-        subject.perform(project.id, :gc, lease_key, lease_uuid)
+        subject.perform(*params)
       end
     end
 
     context 'with no active lease' do
+      let(:params) { [project.id] }
+
       before do
         allow(subject).to receive(:get_lease_uuid).and_return(false)
       end
@@ -63,15 +91,16 @@ RSpec.describe GitGarbageCollectWorker do
           allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid)
         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
-          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(:branch_names).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
 
-          subject.perform(project.id)
+          subject.perform(*params)
         end
 
         context 'when the repository has joined a pool' do
@@ -81,7 +110,7 @@ RSpec.describe GitGarbageCollectWorker do
           it 'ensures the repositories are linked' do
             expect_any_instance_of(PoolRepository).to receive(:link_repository).once
 
-            subject.perform(project.id)
+            subject.perform(*params)
           end
         end
       end
@@ -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(:has_visible_content?).and_call_original
 
-          subject.perform(project.id)
+          subject.perform(*params)
         end
       end
     end
 
     context "repack_full" do
+      let(:task) { :full_repack }
+      let(:gitaly_task) { :repack_full }
+
       before do
         expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
       end
 
-      it "calls Gitaly" do
-        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:repack_full)
-          .and_return(nil)
-
-        subject.perform(project.id, :full_repack, lease_key, lease_uuid)
-      end
+      it_behaves_like 'it calls Gitaly'
+      it_behaves_like 'it updates the project statistics'
     end
 
     context "pack_refs" do
+      let(:task) { :pack_refs }
+      let(:gitaly_task) { :pack_refs }
+
       before do
         expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
       end
 
       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)
 
-        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
 
     context "repack_incremental" do
+      let(:task) { :incremental_repack }
+      let(:gitaly_task) { :repack_incremental }
+
       before do
         expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
       end
 
-      it "calls Gitaly" do
-        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:repack_incremental)
-          .and_return(nil)
-
-        subject.perform(project.id, :incremental_repack, lease_key, lease_uuid)
-      end
+      it_behaves_like 'it calls Gitaly'
+      it_behaves_like 'it updates the project statistics'
     end
 
     shared_examples 'gc tasks' do