Commit 3b4af59a authored by Yorick Peterse's avatar Yorick Peterse

Don't schedule ProjectCacheWorker unless needed

This changes ProjectCacheWorker.perform_async so it only schedules a job
when no lease for the given project is present. This ensures we don't
end up scheduling hundreds of jobs when they won't be executed anyway.
parent 9a677038
...@@ -27,6 +27,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -27,6 +27,7 @@ Please view this file on the master branch, on stable branches it's out of date.
- Fix unauthorized users dragging on issue boards - Fix unauthorized users dragging on issue boards
- Better handle when no users were selected for adding to group or project. (Linus Thiel) - Better handle when no users were selected for adding to group or project. (Linus Thiel)
- Only show register tab if signup enabled. - Only show register tab if signup enabled.
- Only schedule ProjectCacheWorker jobs when needed
## 8.13.0 (2016-10-22) ## 8.13.0 (2016-10-22)
- Removes extra line for empty issue description. (!7045) - Removes extra line for empty issue description. (!7045)
......
...@@ -9,6 +9,18 @@ class ProjectCacheWorker ...@@ -9,6 +9,18 @@ class ProjectCacheWorker
LEASE_TIMEOUT = 15.minutes.to_i LEASE_TIMEOUT = 15.minutes.to_i
def self.lease_for(project_id)
Gitlab::ExclusiveLease.
new("project_cache_worker:#{project_id}", timeout: LEASE_TIMEOUT)
end
# Overwrite Sidekiq's implementation so we only schedule when actually needed.
def self.perform_async(project_id)
# If a lease for this project is still being held there's no point in
# scheduling a new job.
super unless lease_for(project_id).exists?
end
def perform(project_id) def perform(project_id)
if try_obtain_lease_for(project_id) if try_obtain_lease_for(project_id)
Rails.logger. Rails.logger.
...@@ -37,8 +49,6 @@ class ProjectCacheWorker ...@@ -37,8 +49,6 @@ class ProjectCacheWorker
end end
def try_obtain_lease_for(project_id) def try_obtain_lease_for(project_id)
Gitlab::ExclusiveLease. self.class.lease_for(project_id).try_obtain
new("project_cache_worker:#{project_id}", timeout: LEASE_TIMEOUT).
try_obtain
end end
end end
...@@ -48,6 +48,13 @@ module Gitlab ...@@ -48,6 +48,13 @@ module Gitlab
end end
end end
# Returns true if the key for this lease is set.
def exists?
Gitlab::Redis.with do |redis|
redis.exists(redis_key)
end
end
# No #cancel method. See comments above! # No #cancel method. See comments above!
private private
......
require 'spec_helper' require 'spec_helper'
describe Gitlab::ExclusiveLease do 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 it 'cannot obtain twice before the lease has expired' do
lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600) lease = Gitlab::ExclusiveLease.new(unique_key, timeout: 3600)
expect(lease.try_obtain).to eq(true) expect(lease.try_obtain).to eq(true)
...@@ -14,8 +17,20 @@ describe Gitlab::ExclusiveLease do ...@@ -14,8 +17,20 @@ describe Gitlab::ExclusiveLease do
sleep(2 * timeout) # lease should have expired now sleep(2 * timeout) # lease should have expired now
expect(lease.try_obtain).to eq(true) expect(lease.try_obtain).to eq(true)
end end
end
def unique_key describe '#exists?' do
SecureRandom.hex(10) 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
end end
...@@ -50,6 +50,12 @@ RSpec.configure do |config| ...@@ -50,6 +50,12 @@ RSpec.configure do |config|
example.run example.run
Rails.cache = caching_store Rails.cache = caching_store
end end
config.around(:each, :redis) do |example|
Gitlab::Redis.with(&:flushall)
example.run
Gitlab::Redis.with(&:flushall)
end
end end
FactoryGirl::SyntaxRunner.class_eval do FactoryGirl::SyntaxRunner.class_eval do
......
...@@ -5,6 +5,26 @@ describe ProjectCacheWorker do ...@@ -5,6 +5,26 @@ describe ProjectCacheWorker do
subject { described_class.new } 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 describe '#perform' do
context 'when an exclusive lease can be obtained' do context 'when an exclusive lease can be obtained' do
before do before do
......
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