From 05f90b861fbc60cba1912e6a74cb7a4c126e6b7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20=22BKC=22=20Carlb=C3=A4cker?= Date: Wed, 19 Jul 2017 17:34:14 +0200 Subject: Migrate GitGarbageCollectWorker to Gitaly --- app/workers/git_garbage_collect_worker.rb | 39 ++++++++++++++++++- lib/gitlab/gitaly_client/repository_service.rb | 18 ++++++++- spec/workers/git_garbage_collect_worker_spec.rb | 52 ++++++++++++++++++++----- 3 files changed, 97 insertions(+), 12 deletions(-) diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb index d369b639ae9..c95497dfaba 100644 --- a/app/workers/git_garbage_collect_worker.rb +++ b/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) - output, status = Gitlab::Popen.popen(cmd, repo_path) - Gitlab::GitLogger.error("#{description} failed:\n#{output}") unless status.zero? + 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 diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index f5d84ea8762..13e75b256a7 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/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 diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index 309b3172da1..05f971dfd13 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -9,17 +9,51 @@ describe GitGarbageCollectWorker do subject { described_class.new } describe "#perform" do - it "flushes ref caches when the task is 'gc'" do - expect(subject).to receive(:command).with(:gc).and_return([:the, :command]) - expect(Gitlab::Popen).to receive(:popen) - .with([:the, :command], project.repository.path_to_repo).and_return(["", 0]) + 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 + expect_any_instance_of(Repository).to receive(:branch_count).and_call_original + expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original + + 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 - 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_count).and_call_original - expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original + 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) + 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 -- cgit v1.2.1