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

6 files changed, 42 insertions, 15 deletions

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