Use git_garbage_collect_worker to run pack_refs

PackRefs is not an expensive gitaly call - we want to
call it more often (than as part of full `gc`) because
it helps to keep number of refs files small - too many
refs file may be a problem for deployments with
slow storage.

6 files changed, 47 insertions, 4 deletions

diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb
index 10bd5363b51..9428575591e 100644
--- a/app/services/projects/housekeeping_service.rb
+++ b/app/services/projects/housekeeping_service.rb
@@ -11,6 +11,7 @@ module Projects
   class HousekeepingService < BaseService
     # Timeout set to 24h
     LEASE_TIMEOUT = 86400
+    PACK_REFS_PERIOD = 6
 
     class LeaseTaken < StandardError
       def to_s
@@ -76,13 +77,15 @@ module Projects
         :gc
       elsif pushes_since_gc % full_repack_period == 0
         :full_repack
-      else
+      elsif pushes_since_gc % repack_period == 0
         :incremental_repack
+      else
+        :pack_refs
       end
     end
 
     def period_match?
-      [gc_period, full_repack_period, repack_period].any? { |period| pushes_since_gc % period == 0 }
+      [gc_period, full_repack_period, repack_period, PACK_REFS_PERIOD].any? { |period| pushes_since_gc % period == 0 }
     end
 
     def housekeeping_enabled?
diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb
index b33e9b1f718..d4a6f53dae5 100644
--- a/app/workers/git_garbage_collect_worker.rb
+++ b/app/workers/git_garbage_collect_worker.rb
@@ -29,7 +29,7 @@ class GitGarbageCollectWorker
     # 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
+    project.repository.expire_statistics_caches if task != :pack_refs
 
     # In case pack files are deleted, release libgit2 cache and open file
     # descriptors ASAP instead of waiting for Ruby garbage collection
@@ -58,7 +58,12 @@ class GitGarbageCollectWorker
 
   ## `repository` has to be a Gitlab::Git::Repository
   def gitaly_call(task, repository)
-    client = Gitlab::GitalyClient::RepositoryService.new(repository)
+    client = if task == :pack_refs
+               Gitlab::GitalyClient::RefService.new(repository)
+             else
+               Gitlab::GitalyClient::RepositoryService.new(repository)
+             end
+
     case task
     when :gc
       client.garbage_collect(bitmaps_enabled?)
@@ -66,6 +71,8 @@ class GitGarbageCollectWorker
       client.repack_full(bitmaps_enabled?)
     when :incremental_repack
       client.repack_incremental
+    when :pack_refs
+      client.pack_refs
     end
   rescue GRPC::NotFound => e
     Gitlab::GitLogger.error("#{__method__} failed:\nRepository not found")
diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb
index 6f6698607d9..b7d509dfa48 100644
--- a/lib/gitlab/gitaly_client/ref_service.rb
+++ b/lib/gitlab/gitaly_client/ref_service.rb
@@ -239,6 +239,12 @@ module Gitlab
         messages
       end
 
+      def pack_refs
+        request = Gitaly::PackRefsRequest.new(repository: @gitaly_repo)
+
+        GitalyClient.call(@storage, :ref_service, :pack_refs, request)
+      end
+
       private
 
       def consume_refs_response(response)
diff --git a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb
index 0dab39575b9..0bb6e582159 100644
--- a/spec/lib/gitlab/gitaly_client/ref_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/ref_service_spec.rb
@@ -165,4 +165,15 @@ describe Gitlab::GitalyClient::RefService do
       client.delete_refs(except_with_prefixes: prefixes)
     end
   end
+
+  describe '#pack_refs' do
+    it 'sends a pack_refs message' do
+      expect_any_instance_of(Gitaly::RefService::Stub)
+        .to receive(:pack_refs)
+        .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
+        .and_return(double(:pack_refs_response))
+
+      client.pack_refs
+    end
+  end
 end
diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb
index 368cf123c3e..f651db70cbd 100644
--- a/spec/services/projects/housekeeping_service_spec.rb
+++ b/spec/services/projects/housekeeping_service_spec.rb
@@ -81,6 +81,9 @@ describe Projects::HousekeepingService do
         # At push 10, 20, ... (except those above)
         expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :incremental_repack, :the_lease_key, :the_uuid)
           .exactly(16).times
+        # At push 6, 12, 18, ... (except those above)
+        expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :pack_refs, :the_lease_key, :the_uuid)
+          .exactly(27).times
 
         201.times do
           subject.increment!
diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb
index 2459638c3e6..cc1c23bb9e7 100644
--- a/spec/workers/git_garbage_collect_worker_spec.rb
+++ b/spec/workers/git_garbage_collect_worker_spec.rb
@@ -115,6 +115,19 @@ describe GitGarbageCollectWorker do
       end
     end
 
+    context "pack_refs" do
+      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)
+          .and_return(nil)
+
+        subject.perform(project.id, :pack_refs, lease_key, lease_uuid)
+      end
+    end
+
     context "repack_incremental" do
       before do
         expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)