Commit 97999fd4 authored by Jacob Vosmaer (GitLab)'s avatar Jacob Vosmaer (GitLab)

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
parents 179be71d 3dc6bf2b
...@@ -4,6 +4,7 @@ v 8.10.0 (unreleased) ...@@ -4,6 +4,7 @@ v 8.10.0 (unreleased)
- Expose {should,force}_remove_source_branch (Ben Boeckel) - Expose {should,force}_remove_source_branch (Ben Boeckel)
- Fix commit builds API, return all builds for all pipelines for given commit. !4849 - Fix commit builds API, return all builds for all pipelines for given commit. !4849
- Replace Haml with Hamlit to make view rendering faster. !3666 - 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 - Refactor repository paths handling to allow multiple git mount points
- Optimize system note visibility checking by memoizing the visible reference count !5070 - Optimize system note visibility checking by memoizing the visible reference count !5070
- Add Application Setting to configure default Repository Path for new projects - Add Application Setting to configure default Repository Path for new projects
......
...@@ -7,8 +7,6 @@ ...@@ -7,8 +7,6 @@
# #
module Projects module Projects
class HousekeepingService < BaseService class HousekeepingService < BaseService
include Gitlab::ShellAdapter
LEASE_TIMEOUT = 3600 LEASE_TIMEOUT = 3600
class LeaseTaken < StandardError class LeaseTaken < StandardError
...@@ -24,7 +22,7 @@ module Projects ...@@ -24,7 +22,7 @@ module Projects
def execute def execute
raise LeaseTaken unless try_obtain_lease raise LeaseTaken unless try_obtain_lease
GitlabShellOneShotWorker.perform_async(:gc, @project.repository_storage_path, @project.path_with_namespace) GitGarbageCollectWorker.perform_async(@project.id)
ensure ensure
Gitlab::Metrics.measure(:reset_pushes_since_gc) do Gitlab::Metrics.measure(:reset_pushes_since_gc) do
update_pushes_since_gc(0) update_pushes_since_gc(0)
......
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
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
...@@ -12,7 +12,7 @@ describe Projects::HousekeepingService do ...@@ -12,7 +12,7 @@ describe Projects::HousekeepingService do
it 'enqueues a sidekiq job' do it 'enqueues a sidekiq job' do
expect(subject).to receive(:try_obtain_lease).and_return(true) 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 subject.execute
expect(project.pushes_since_gc).to eq(0) expect(project.pushes_since_gc).to eq(0)
...@@ -20,7 +20,7 @@ describe Projects::HousekeepingService do ...@@ -20,7 +20,7 @@ describe Projects::HousekeepingService do
it 'does not enqueue a job when no lease can be obtained' 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(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 { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
expect(project.pushes_since_gc).to eq(0) expect(project.pushes_since_gc).to eq(0)
......
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment