diff options
author | Jacob Vosmaer (GitLab) <jacob@gitlab.com> | 2016-07-12 13:24:42 +0000 |
---|---|---|
committer | Jacob Vosmaer (GitLab) <jacob@gitlab.com> | 2016-07-12 13:24:42 +0000 |
commit | 97999fd4203846ad807de18eab5d7a2176344ce1 (patch) | |
tree | 5d6779122db6c1af0040e3ae928ae66b7263a2c6 | |
parent | 179be71dc5f8c11c1ad2f668b2e681b29ca333a1 (diff) | |
parent | 3dc6bf2b71f995a3b6ca40ebbf9abb5c11397b8b (diff) | |
download | gitlab-ce-97999fd4203846ad807de18eab5d7a2176344ce1.tar.gz |
Merge branch 'expire-branch-cache-after-gc' into 'master'
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.
I'm not totally convinced this is the right solution, but since we and our customers are experiencing this issue quite frequently, I'm taking a stab at it.
Possible workaround for #15392
See merge request !5160
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | app/services/projects/housekeeping_service.rb | 4 | ||||
-rw-r--r-- | app/workers/git_garbage_collect_worker.rb | 14 | ||||
-rw-r--r-- | app/workers/gitlab_shell_one_shot_worker.rb | 10 | ||||
-rw-r--r-- | spec/services/projects/housekeeping_service_spec.rb | 4 | ||||
-rw-r--r-- | spec/workers/git_garbage_collect_worker_spec.rb | 24 |
6 files changed, 42 insertions, 15 deletions
diff --git a/CHANGELOG b/CHANGELOG index f12f6423c7a..ee3ee4c37d6 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 |