From 3dc6bf2b71f995a3b6ca40ebbf9abb5c11397b8b Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 8 Jul 2016 12:32:58 -0700 Subject: Expire the branch cache after `git gc` runs Due to a stale NFS cache, it's possible that a branch lookup fails while `git gc` is running and causes missing branches in merge requests. Possible workaround for #15392 --- CHANGELOG | 1 + app/services/projects/housekeeping_service.rb | 4 +--- app/workers/git_garbage_collect_worker.rb | 14 +++++++++++++ app/workers/gitlab_shell_one_shot_worker.rb | 10 --------- .../services/projects/housekeeping_service_spec.rb | 4 ++-- spec/workers/git_garbage_collect_worker_spec.rb | 24 ++++++++++++++++++++++ 6 files changed, 42 insertions(+), 15 deletions(-) create mode 100644 app/workers/git_garbage_collect_worker.rb delete mode 100644 app/workers/gitlab_shell_one_shot_worker.rb create mode 100644 spec/workers/git_garbage_collect_worker_spec.rb diff --git a/CHANGELOG b/CHANGELOG index 84bfc27b151..d77d02f82c3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.10.0 (unreleased) - Expose {should,force}_remove_source_branch (Ben Boeckel) - Fix commit builds API, return all builds for all pipelines for given commit. !4849 - Replace Haml with Hamlit to make view rendering faster. !3666 + - Expire the branch cache after `git gc` runs - Refactor repository paths handling to allow multiple git mount points - Optimize system note visibility checking by memoizing the visible reference count !5070 - Add Application Setting to configure default Repository Path for new projects diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb index 752c11d7ae6..c9ad710b7bf 100644 --- a/app/services/projects/housekeeping_service.rb +++ b/app/services/projects/housekeeping_service.rb @@ -7,8 +7,6 @@ # module Projects class HousekeepingService < BaseService - include Gitlab::ShellAdapter - LEASE_TIMEOUT = 3600 class LeaseTaken < StandardError @@ -24,7 +22,7 @@ module Projects def execute raise LeaseTaken unless try_obtain_lease - GitlabShellOneShotWorker.perform_async(:gc, @project.repository_storage_path, @project.path_with_namespace) + GitGarbageCollectWorker.perform_async(@project.id) ensure Gitlab::Metrics.measure(:reset_pushes_since_gc) do update_pushes_since_gc(0) diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb new file mode 100644 index 00000000000..2fa3c838f55 --- /dev/null +++ b/app/workers/git_garbage_collect_worker.rb @@ -0,0 +1,14 @@ +class GitGarbageCollectWorker + include Sidekiq::Worker + include Gitlab::ShellAdapter + + sidekiq_options queue: :gitlab_shell, retry: false + + def perform(project_id) + project = Project.find(project_id) + + gitlab_shell.gc(project.repository_storage_path, project.path_with_namespace) + # Expire the branch cache in case garbage collection caused a ref lookup to fail + project.repository.after_create_branch + end +end diff --git a/app/workers/gitlab_shell_one_shot_worker.rb b/app/workers/gitlab_shell_one_shot_worker.rb deleted file mode 100644 index 4ddbcf574d5..00000000000 --- a/app/workers/gitlab_shell_one_shot_worker.rb +++ /dev/null @@ -1,10 +0,0 @@ -class GitlabShellOneShotWorker - include Sidekiq::Worker - include Gitlab::ShellAdapter - - sidekiq_options queue: :gitlab_shell, retry: false - - def perform(action, *arg) - gitlab_shell.send(action, *arg) - end -end diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb index bd4dc6a0f79..7ab95e042ce 100644 --- a/spec/services/projects/housekeeping_service_spec.rb +++ b/spec/services/projects/housekeeping_service_spec.rb @@ -12,7 +12,7 @@ describe Projects::HousekeepingService do it 'enqueues a sidekiq job' do expect(subject).to receive(:try_obtain_lease).and_return(true) - expect(GitlabShellOneShotWorker).to receive(:perform_async).with(:gc, project.repository_storage_path, project.path_with_namespace) + expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id) subject.execute expect(project.pushes_since_gc).to eq(0) @@ -20,7 +20,7 @@ describe Projects::HousekeepingService do it 'does not enqueue a job when no lease can be obtained' do expect(subject).to receive(:try_obtain_lease).and_return(false) - expect(GitlabShellOneShotWorker).not_to receive(:perform_async) + expect(GitGarbageCollectWorker).not_to receive(:perform_async) expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken) expect(project.pushes_since_gc).to eq(0) diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb new file mode 100644 index 00000000000..a9cce8b8b59 --- /dev/null +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe GitGarbageCollectWorker do + let(:project) { create(:project) } + let(:shell) { Gitlab::Shell.new } + + subject { GitGarbageCollectWorker.new } + + before do + allow(subject).to receive(:gitlab_shell).and_return(shell) + end + + describe "#perform" do + it "runs `git gc`" do + expect(shell).to receive(:gc).with( + project.repository_storage_path, + project.path_with_namespace). + and_return(true) + expect_any_instance_of(Repository).to receive(:after_create_branch) + + subject.perform(project.id) + end + end +end -- cgit v1.2.1