Adds exclusive lease to Git garbage collect worker.35558-only-one-garbage-collection-should-be-running-per-project-at-once

3 files changed, 114 insertions, 20 deletions

diff --git a/spec/lib/gitlab/exclusive_lease_spec.rb b/spec/lib/gitlab/exclusive_lease_spec.rb
index c1ed47cf64a..7322a326b01 100644
--- a/spec/lib/gitlab/exclusive_lease_spec.rb
+++ b/spec/lib/gitlab/exclusive_lease_spec.rb
@@ -47,6 +47,18 @@ describe Gitlab::ExclusiveLease, :clean_gitlab_redis_shared_state do
     end
   end
 
+  describe '.get_uuid' do
+    it 'gets the uuid if lease with the key associated exists' do
+      uuid = described_class.new(unique_key, timeout: 3600).try_obtain
+
+      expect(described_class.get_uuid(unique_key)).to eq(uuid)
+    end
+
+    it 'returns false if the lease does not exist' do
+      expect(described_class.get_uuid(unique_key)).to be false
+    end
+  end
+
   describe '.cancel' do
     it 'can cancel a lease' do
       uuid = new_lease(unique_key)
diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb
index 9386c110385..437c009e7fa 100644
--- a/spec/services/projects/housekeeping_service_spec.rb
+++ b/spec/services/projects/housekeeping_service_spec.rb
@@ -20,6 +20,7 @@ describe Projects::HousekeepingService do
       expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :the_task, :the_lease_key, :the_uuid)
 
       subject.execute
+
       expect(project.reload.pushes_since_gc).to eq(0)
     end
 
diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb
index c4979792194..6f9ddb6c63c 100644
--- a/spec/workers/git_garbage_collect_worker_spec.rb
+++ b/spec/workers/git_garbage_collect_worker_spec.rb
@@ -5,28 +5,100 @@ require 'spec_helper'
 describe GitGarbageCollectWorker do
   let(:project) { create(:project, :repository) }
   let(:shell) { Gitlab::Shell.new }
+  let!(:lease_uuid) { SecureRandom.uuid }
+  let!(:lease_key) { "project_housekeeping:#{project.id}" }
 
   subject { described_class.new }
 
   describe "#perform" do
     shared_examples 'flushing ref caches' do |gitaly|
-      it "flushes ref caches when the task if 'gc'" do
-        expect(subject).to receive(:command).with(:gc).and_return([:the, :command])
-
-        if gitaly
-          expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect)
-            .and_return(nil)
-        else
-          expect(Gitlab::Popen).to receive(:popen)
-            .with([:the, :command], project.repository.path_to_repo).and_return(["", 0])
+      context 'with active lease_uuid' do
+        before do
+          allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
         end
 
-        expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
-        expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
-        expect_any_instance_of(Gitlab::Git::Repository).to receive(:branch_count).and_call_original
-        expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
+        it "flushes ref caches when the task if 'gc'" do
+          expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original
+          expect(subject).to receive(:command).with(:gc).and_return([:the, :command])
 
-        subject.perform(project.id)
+          if gitaly
+            expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect)
+              .and_return(nil)
+          else
+            expect(Gitlab::Popen).to receive(:popen)
+              .with([:the, :command], project.repository.path_to_repo).and_return(["", 0])
+          end
+
+          expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
+          expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
+          expect_any_instance_of(Gitlab::Git::Repository).to receive(:branch_count).and_call_original
+          expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
+
+          subject.perform(project.id, :gc, lease_key, lease_uuid)
+        end
+      end
+
+      context 'with different lease than the active one' do
+        before do
+          allow(subject).to receive(:get_lease_uuid).and_return(SecureRandom.uuid)
+        end
+
+        it 'returns silently' do
+          expect(subject).not_to receive(:command)
+          expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original
+          expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
+          expect_any_instance_of(Repository).not_to receive(:branch_count).and_call_original
+          expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
+
+          subject.perform(project.id, :gc, lease_key, lease_uuid)
+        end
+      end
+
+      context 'with no active lease' do
+        before do
+          allow(subject).to receive(:get_lease_uuid).and_return(false)
+        end
+
+        context 'when is able to get the lease' do
+          before do
+            allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid)
+          end
+
+          it "flushes ref caches when the task if 'gc'" do
+            expect(subject).to receive(:command).with(:gc).and_return([:the, :command])
+
+            if gitaly
+              expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect)
+                .and_return(nil)
+            else
+              expect(Gitlab::Popen).to receive(:popen)
+                .with([:the, :command], project.repository.path_to_repo).and_return(["", 0])
+            end
+
+            expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original
+            expect_any_instance_of(Repository).to receive(:branch_names).and_call_original
+            expect_any_instance_of(Gitlab::Git::Repository).to receive(:branch_count).and_call_original
+            expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original
+
+            subject.perform(project.id)
+          end
+        end
+
+        context 'when no lease can be obtained' do
+          before do
+            expect(subject).to receive(:try_obtain_lease).and_return(false)
+          end
+
+          it 'returns silently' do
+            expect(subject).not_to receive(:command)
+            expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original
+            expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original
+            expect_any_instance_of(Repository).not_to receive(:branch_count).and_call_original
+            expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original
+
+            subject.perform(project.id)
+          end
+        end
       end
     end
 
@@ -39,25 +111,34 @@ describe GitGarbageCollectWorker do
     end
 
     context "repack_full" do
+      before do
+        expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
+      end
+
       it "calls Gitaly" do
         expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:repack_full)
           .and_return(nil)
 
-        subject.perform(project.id, :full_repack)
+        subject.perform(project.id, :full_repack, lease_key, lease_uuid)
       end
     end
 
     context "repack_incremental" do
+      before do
+        expect(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
+      end
+
       it "calls Gitaly" do
         expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:repack_incremental)
           .and_return(nil)
 
-        subject.perform(project.id, :incremental_repack)
+        subject.perform(project.id, :incremental_repack, lease_key, lease_uuid)
       end
     end
 
     shared_examples 'gc tasks' do
       before do
+        allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid)
         allow(subject).to receive(:bitmaps_enabled?).and_return(bitmaps_enabled)
       end
 
@@ -67,7 +148,7 @@ describe GitGarbageCollectWorker do
 
         expect(before_packs.count).to be >= 1
 
-        subject.perform(project.id, 'incremental_repack')
+        subject.perform(project.id, 'incremental_repack', lease_key, lease_uuid)
         after_packs = packs(project)
 
         # Exactly one new pack should have been created
@@ -79,12 +160,12 @@ describe GitGarbageCollectWorker do
 
       it 'full repack consolidates into 1 packfile' do
         create_objects(project)
-        subject.perform(project.id, 'incremental_repack')
+        subject.perform(project.id, 'incremental_repack', lease_key, lease_uuid)
         before_packs = packs(project)
 
         expect(before_packs.count).to be >= 2
 
-        subject.perform(project.id, 'full_repack')
+        subject.perform(project.id, 'full_repack', lease_key, lease_uuid)
         after_packs = packs(project)
 
         expect(after_packs.count).to eq(1)
@@ -102,7 +183,7 @@ describe GitGarbageCollectWorker do
 
         expect(before_packs.count).to be >= 1
 
-        subject.perform(project.id, 'gc')
+        subject.perform(project.id, 'gc', lease_key, lease_uuid)
         after_packed_refs = packed_refs(project)
         after_packs = packs(project)