diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2019-08-20 21:09:04 +0200 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2019-08-21 12:06:10 +0200 |
commit | 3683d2d251889865334f861791e5e743dca48898 (patch) | |
tree | d17ab39389c1e9e009a0a9e7854aa8dcf7cfaab3 | |
parent | c2cbfc5c4afbe8385659f97769db8450284639cf (diff) | |
download | gitlab-ce-3683d2d251889865334f861791e5e743dca48898.tar.gz |
Perform cheap thread find
If we process message that is not designated to us
previously we would fire a separate Thread for that.
We don't need to do it. We can cheaply check if thread
is available, if it is, we can perform expensive operation
then.
-rw-r--r-- | lib/gitlab/sidekiq_monitor.rb | 25 | ||||
-rw-r--r-- | spec/lib/gitlab/sidekiq_monitor_spec.rb | 4 |
2 files changed, 17 insertions, 12 deletions
diff --git a/lib/gitlab/sidekiq_monitor.rb b/lib/gitlab/sidekiq_monitor.rb index 0d4709508a0..bfbf998b3a0 100644 --- a/lib/gitlab/sidekiq_monitor.rb +++ b/lib/gitlab/sidekiq_monitor.rb @@ -110,11 +110,13 @@ module Gitlab def process_job_cancel(jid) return unless jid - # since this might take time, process cancel in a new thread - Thread.new do - find_thread(jid) do |thread| - next unless thread + # try to find thread without lock + return unless find_thread_unsafe(jid) + Thread.new do + # try to find a thread, but with guaranteed + # handle that this thread corresponds to actually running job + find_thread_with_lock(jid) do |thread| Sidekiq.logger.warn( class: self.class, action: 'cancel', @@ -130,13 +132,18 @@ module Gitlab # This method needs to be thread-safe # This is why it passes thread in block, # to ensure that we do process this thread - def find_thread(jid) - return unless jid + def find_thread_unsafe(jid) + jobs_thread[jid] + end + + def find_thread_with_lock(jid) + # don't try to lock if we cannot find the thread + return unless find_thread_unsafe(jid) jobs_mutex.synchronize do - thread = jobs_thread[jid] - yield(thread) - thread + find_thread_unsafe(jid).tap do |thread| + yield(thread) if thread + end end end diff --git a/spec/lib/gitlab/sidekiq_monitor_spec.rb b/spec/lib/gitlab/sidekiq_monitor_spec.rb index 7c9fc598b02..f1804bd0755 100644 --- a/spec/lib/gitlab/sidekiq_monitor_spec.rb +++ b/spec/lib/gitlab/sidekiq_monitor_spec.rb @@ -145,9 +145,7 @@ describe Gitlab::SidekiqMonitor do context 'when jid is not found' do it 'does not log cancellation message' do expect(Sidekiq.logger).not_to receive(:warn) - expect(subject).to be_a(Thread) - - subject.join + expect(subject).to be_nil end end |