From e03602e09de5d72b3bc5aee0df2a42807456a391 Mon Sep 17 00:00:00 2001
From: Zeger-Jan van de Weg <git@zjvandeweg.nl>
Date: Tue, 8 Jan 2019 13:02:58 +0100
Subject: [PATCH] Ensure pool participants are linked before GC

In theory the case could happen that the initial linking of the pool
fails and so do all the retries that Sidekiq performs. This could lead
to data loss.

To prevent that case, linking is done before Gits GC too. This makes
sure that case doesn't happen.
---
 app/models/project.rb                           |  4 ++++
 app/workers/git_garbage_collect_worker.rb       |  1 +
 app/workers/object_pool/join_worker.rb          | 11 +++++------
 spec/workers/git_garbage_collect_worker_spec.rb | 11 +++++++++++
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index 7ab2fc30c24..27be16720b5 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -2039,6 +2039,10 @@ class Project < ActiveRecord::Base
     pool_repository&.unlink_repository(repository) && update_column(:pool_repository_id, nil)
   end
 
+  def link_pool_repository
+    pool_repository&.link_repository(repository)
+  end
+
   private
 
   def merge_requests_allowing_collaboration(source_branch = nil)
diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb
index d3628b23189..b33e9b1f718 100644
--- a/app/workers/git_garbage_collect_worker.rb
+++ b/app/workers/git_garbage_collect_worker.rb
@@ -23,6 +23,7 @@ class GitGarbageCollectWorker
     end
 
     task = task.to_sym
+    project.link_pool_repository
     gitaly_call(task, project.repository.raw_repository)
 
     # Refresh the branch cache in case garbage collection caused a ref lookup to fail
diff --git a/app/workers/object_pool/join_worker.rb b/app/workers/object_pool/join_worker.rb
index 07676011b2a..9c5161fd55a 100644
--- a/app/workers/object_pool/join_worker.rb
+++ b/app/workers/object_pool/join_worker.rb
@@ -5,14 +5,13 @@ module ObjectPool
     include ApplicationWorker
     include ObjectPoolQueue
 
-    def perform(pool_id, project_id)
-      pool = PoolRepository.find_by_id(pool_id)
-      return unless pool&.joinable?
-
+    # The use of pool id is deprecated. Keeping the argument allows old jobs to
+    # still be performed.
+    def perform(_pool_id, project_id)
       project = Project.find_by_id(project_id)
-      return unless project
+      return unless project&.pool_repository&.joinable?
 
-      pool.link_repository(project.repository)
+      project.link_pool_repository
 
       Projects::HousekeepingService.new(project).execute
     end
diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb
index a159f24f876..4895a968d6e 100644
--- a/spec/workers/git_garbage_collect_worker_spec.rb
+++ b/spec/workers/git_garbage_collect_worker_spec.rb
@@ -71,6 +71,17 @@ describe GitGarbageCollectWorker do
 
           subject.perform(project.id)
         end
+
+        context 'when the repository has joined a pool' do
+          let!(:pool) { create(:pool_repository, :ready) }
+          let(:project) { pool.source_project }
+
+          it 'ensures the repositories are linked' do
+            expect_any_instance_of(PoolRepository).to receive(:link_repository).once
+
+            subject.perform(project.id)
+          end
+        end
       end
 
       context 'when no lease can be obtained' do