Commit 3683d2d2 authored by Kamil Trzciński's avatar Kamil Trzciński

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.
parent c2cbfc5c
...@@ -110,11 +110,13 @@ module Gitlab ...@@ -110,11 +110,13 @@ module Gitlab
def process_job_cancel(jid) def process_job_cancel(jid)
return unless jid return unless jid
# since this might take time, process cancel in a new thread # try to find thread without lock
Thread.new do return unless find_thread_unsafe(jid)
find_thread(jid) do |thread|
next unless thread
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( Sidekiq.logger.warn(
class: self.class, class: self.class,
action: 'cancel', action: 'cancel',
...@@ -130,13 +132,18 @@ module Gitlab ...@@ -130,13 +132,18 @@ module Gitlab
# This method needs to be thread-safe # This method needs to be thread-safe
# This is why it passes thread in block, # This is why it passes thread in block,
# to ensure that we do process this thread # to ensure that we do process this thread
def find_thread(jid) def find_thread_unsafe(jid)
return unless 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 jobs_mutex.synchronize do
thread = jobs_thread[jid] find_thread_unsafe(jid).tap do |thread|
yield(thread) yield(thread) if thread
thread end
end end
end end
......
...@@ -145,9 +145,7 @@ describe Gitlab::SidekiqMonitor do ...@@ -145,9 +145,7 @@ describe Gitlab::SidekiqMonitor do
context 'when jid is not found' do context 'when jid is not found' do
it 'does not log cancellation message' do it 'does not log cancellation message' do
expect(Sidekiq.logger).not_to receive(:warn) expect(Sidekiq.logger).not_to receive(:warn)
expect(subject).to be_a(Thread) expect(subject).to be_nil
subject.join
end end
end end
......
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