Merge branch 'gitaly-398-git-garbage-collect' into 'master'

Migrate GitGarbageCollectWorker to Gitaly

Closes gitaly#398

See merge request !12975

app/workers/git_garbage_collect_worker.rb

@@ -5,6 +5,12 @@ class GitGarbageCollectWorker
 
   sidekiq_options retry: false
 
+  GITALY_MIGRATED_TASKS = {
+    gc: :garbage_collect,
+    full_repack: :repack_full,
+    incremental_repack: :repack_incremental
+  }.freeze
+
   def perform(project_id, task = :gc, lease_key = nil, lease_uuid = nil)
     project = Project.find(project_id)
     task = task.to_sym
@@ -15,8 +21,14 @@ class GitGarbageCollectWorker
 
     Gitlab::GitLogger.info(description)
 
+    gitaly_migrate(GITALY_MIGRATED_TASKS[task]) do |is_enabled|
+      if is_enabled
+        gitaly_call(task, project.repository.raw_repository)
+      else
         output, status = Gitlab::Popen.popen(cmd, repo_path)
         Gitlab::GitLogger.error("#{description} failed:\n#{output}") unless status.zero?
+      end
+    end
 
     # Refresh the branch cache in case garbage collection caused a ref lookup to fail
     flush_ref_caches(project) if task == :gc
@@ -26,6 +38,19 @@ class GitGarbageCollectWorker
 
   private
 
+  ## `repository` has to be a Gitlab::Git::Repository
+  def gitaly_call(task, repository)
+    client = Gitlab::GitalyClient::RepositoryService.new(repository)
+    case task
+    when :gc
+      client.garbage_collect(bitmaps_enabled?)
+    when :full_repack
+      client.repack_full(bitmaps_enabled?)
+    when :incremental_repack
+      client.repack_incremental
+    end
+  end
+
   def command(task)
     case task
     when :gc
@@ -55,4 +80,14 @@ class GitGarbageCollectWorker
     config_value = write_bitmaps ? 'true' : 'false'
     %W[git -c repack.writeBitmaps=#{config_value}]
   end
+
+  def gitaly_migrate(method, &block)
+    Gitlab::GitalyClient.migrate(method, &block)
+  rescue GRPC::NotFound => e
+    Gitlab::GitLogger.error("#{method} failed:\nRepository not found")
+    raise Gitlab::Git::Repository::NoRepository.new(e)
+  rescue GRPC::BadStatus => e
+    Gitlab::GitLogger.error("#{method} failed:\n#{e}")
+    raise Gitlab::Git::CommandError.new(e)
+  end
 end

lib/gitlab/gitaly_client/repository_service.rb

@@ -4,12 +4,28 @@ module Gitlab
       def initialize(repository)
         @repository = repository
         @gitaly_repo = repository.gitaly_repository
+        @storage = repository.storage
       end
 
       def exists?
         request = Gitaly::RepositoryExistsRequest.new(repository: @gitaly_repo)
 
-        GitalyClient.call(@repository.storage, :repository_service, :exists, request).exists
+        GitalyClient.call(@storage, :repository_service, :exists, request).exists
+      end
+
+      def garbage_collect(create_bitmap)
+        request = Gitaly::GarbageCollectRequest.new(repository: @gitaly_repo, create_bitmap: create_bitmap)
+        GitalyClient.call(@storage, :repository_service, :garbage_collect, request)
+      end
+
+      def repack_full(create_bitmap)
+        request = Gitaly::RepackFullRequest.new(repository: @gitaly_repo, create_bitmap: create_bitmap)
+        GitalyClient.call(@storage, :repository_service, :repack_full, request)
+      end
+
+      def repack_incremental
+        request = Gitaly::RepackIncrementalRequest.new(repository: @gitaly_repo)
+        GitalyClient.call(@storage, :repository_service, :repack_incremental, request)
       end
     end
   end

spec/workers/git_garbage_collect_worker_spec.rb

@@ -9,10 +9,17 @@ describe GitGarbageCollectWorker do
   subject { described_class.new }
 
   describe "#perform" do
-    it "flushes ref caches when the task is 'gc'" do
+    shared_examples 'flushing ref caches' do |gitaly|
+      it "flushes ref caches when the task if 'gc'" do
         expect(subject).to receive(:command).with(:gc).and_return([:the, :command])
+
+        if gitaly
+          expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect)
+            .and_return(nil)
+        else
           expect(Gitlab::Popen).to receive(:popen)
             .with([:the, :command], project.repository.path_to_repo).and_return(["", 0])
+        end
 
         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
@@ -21,6 +28,33 @@ describe GitGarbageCollectWorker do
 
         subject.perform(project.id)
       end
+    end
+
+    context "with Gitaly turned on" do
+      it_should_behave_like 'flushing ref caches', true
+    end
+
+    context "with Gitaly turned off", skip_gitaly_mock: true do
+      it_should_behave_like 'flushing ref caches', false
+    end
+
+    context "repack_full" do
+      it "calls Gitaly" do
+        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:repack_full)
+          .and_return(nil)
+
+        subject.perform(project.id, :full_repack)
+      end
+    end
+
+    context "repack_incremental" do
+      it "calls Gitaly" do
+        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:repack_incremental)
+          .and_return(nil)
+
+        subject.perform(project.id, :incremental_repack)
+      end
+    end
 
     shared_examples 'gc tasks' do
       before do