Merge branch 'project-cache-worker-scheduling' into 'master'

Don't schedule ProjectCacheWorker unless needed

This MR changes `ProjectCacheWorker.perform_async` so scheduling only takes place when needed. See the commits for more details.

See merge request !7099

Signed-off-by: Rémy Coutable <remy@rymai.me>

3 files changed, 55 insertions, 14 deletions

diff --git a/spec/lib/gitlab/exclusive_lease_spec.rb b/spec/lib/gitlab/exclusive_lease_spec.rb
index fbdb7ea34ac..6b3bd08b978 100644
--- a/spec/lib/gitlab/exclusive_lease_spec.rb
+++ b/spec/lib/gitlab/exclusive_lease_spec.rb
@@ -1,21 +1,36 @@
 require 'spec_helper'
 
-describe Gitlab::ExclusiveLease do
-  it 'cannot obtain twice before the lease has expired' do
-    lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600)
-    expect(lease.try_obtain).to eq(true)
-    expect(lease.try_obtain).to eq(false)
-  end
+describe Gitlab::ExclusiveLease, type: :redis do
+  let(:unique_key) { SecureRandom.hex(10) }
+
+  describe '#try_obtain' do
+    it 'cannot obtain twice before the lease has expired' do
+      lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600)
+      expect(lease.try_obtain).to eq(true)
+      expect(lease.try_obtain).to eq(false)
+    end
 
-  it 'can obtain after the lease has expired' do
-    timeout = 1
-    lease = Gitlab::ExclusiveLease.new(unique_key, timeout: timeout)
-    lease.try_obtain # start the lease
-    sleep(2 * timeout) # lease should have expired now
-    expect(lease.try_obtain).to eq(true)
+    it 'can obtain after the lease has expired' do
+      timeout = 1
+      lease = Gitlab::ExclusiveLease.new(unique_key, timeout: timeout)
+      lease.try_obtain # start the lease
+      sleep(2 * timeout) # lease should have expired now
+      expect(lease.try_obtain).to eq(true)
+    end
   end
 
-  def unique_key
-    SecureRandom.hex(10)
+  describe '#exists?' do
+    it 'returns true for an existing lease' do
+      lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600)
+      lease.try_obtain
+
+      expect(lease.exists?).to eq(true)
+    end
+
+    it 'returns false for a lease that does not exist' do
+      lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600)
+
+      expect(lease.exists?).to eq(false)
+    end
   end
 end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index b19f5824236..06d52f0f735 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -50,6 +50,12 @@ RSpec.configure do |config|
     example.run
     Rails.cache = caching_store
   end
+
+  config.around(:each, :redis) do |example|
+    Gitlab::Redis.with(&:flushall)
+    example.run
+    Gitlab::Redis.with(&:flushall)
+  end
 end
 
 FactoryGirl::SyntaxRunner.class_eval do
diff --git a/spec/workers/project_cache_worker_spec.rb b/spec/workers/project_cache_worker_spec.rb
index f5b60b90d11..bfa8c0ff2c6 100644
--- a/spec/workers/project_cache_worker_spec.rb
+++ b/spec/workers/project_cache_worker_spec.rb
@@ -5,6 +5,26 @@ describe ProjectCacheWorker do
 
   subject { described_class.new }
 
+  describe '.perform_async' do
+    it 'schedules the job when no lease exists' do
+      allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:exists?).
+        and_return(false)
+
+      expect_any_instance_of(described_class).to receive(:perform)
+
+      described_class.perform_async(project.id)
+    end
+
+    it 'does not schedule the job when a lease exists' do
+      allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:exists?).
+        and_return(true)
+
+      expect_any_instance_of(described_class).not_to receive(:perform)
+
+      described_class.perform_async(project.id)
+    end
+  end
+
   describe '#perform' do
     context 'when an exclusive lease can be obtained' do
       before do