Properly handle `sidekiq` skip

Transform `CancelledError` into `JobRetry::Skip`
This commit is contained in:
Kamil Trzciński 2019-08-21 12:03:42 +02:00
parent cb193cd6b5
commit 8d17c4dae6
5 changed files with 31 additions and 17 deletions

View File

@ -28,12 +28,13 @@ if Rails.env.development?
end
enable_json_logs = Gitlab.config.sidekiq.log_format == 'json'
enable_sidekiq_monitor = ENV.fetch("SIDEKIQ_MONITOR_WORKER", 0).to_i.nonzero?
Sidekiq.configure_server do |config|
config.redis = queues_config_hash
config.server_middleware do |chain|
chain.add Gitlab::SidekiqMiddleware::Monitor
chain.add Gitlab::SidekiqMiddleware::Monitor if enable_sidekiq_monitor
chain.add Gitlab::SidekiqMiddleware::Metrics if Settings.monitoring.sidekiq_exporter
chain.add Gitlab::SidekiqMiddleware::ArgumentsLogger if ENV['SIDEKIQ_LOG_ARGUMENTS'] && !enable_json_logs
chain.add Gitlab::SidekiqMiddleware::MemoryKiller if ENV['SIDEKIQ_MEMORY_KILLER_MAX_RSS']
@ -59,9 +60,7 @@ Sidekiq.configure_server do |config|
# Sidekiq (e.g. in an initializer).
ActiveRecord::Base.clear_all_connections!
if ENV.fetch("SIDEKIQ_MONITOR_WORKER", 0).to_i.nonzero?
Gitlab::SidekiqMonitor.instance.start
end
Gitlab::SidekiqMonitor.instance.start if enable_sidekiq_monitor
end
if enable_reliable_fetch?

View File

@ -7,6 +7,9 @@ module Gitlab
Gitlab::SidekiqMonitor.instance.within_job(job['jid'], queue) do
yield
end
rescue Gitlab::SidekiqMonitor::CancelledError
# ignore retries
raise Sidekiq::JobRetry::Skip
end
end
end

View File

@ -30,7 +30,7 @@ module Gitlab
if cancelled?(jid)
Sidekiq.logger.warn(
class: self.class,
class: self.class.to_s,
action: 'run',
queue: queue,
jid: jid,
@ -62,7 +62,7 @@ module Gitlab
def start_working
Sidekiq.logger.info(
class: self.class,
class: self.class.to_s,
action: 'start',
message: 'Starting Monitor Daemon'
)
@ -74,7 +74,7 @@ module Gitlab
ensure
Sidekiq.logger.warn(
class: self.class,
class: self.class.to_s,
action: 'stop',
message: 'Stopping Monitor Daemon'
)
@ -94,7 +94,7 @@ module Gitlab
end
rescue Exception => e # rubocop:disable Lint/RescueException
Sidekiq.logger.warn(
class: self.class,
class: self.class.to_s,
action: 'exception',
message: e.message
)
@ -105,7 +105,7 @@ module Gitlab
def process_message(message)
Sidekiq.logger.info(
class: self.class,
class: self.class.to_s,
channel: NOTIFICATION_CHANNEL,
message: 'Received payload on channel',
payload: message
@ -139,7 +139,7 @@ module Gitlab
# running job
find_thread_with_lock(jid) do |thread|
Sidekiq.logger.warn(
class: self.class,
class: self.class.to_s,
action: 'cancel',
message: 'Canceling thread with CancelledError',
jid: jid,

View File

@ -25,5 +25,17 @@ describe Gitlab::SidekiqMiddleware::Monitor do
expect(result).to eq('value')
end
context 'when cancel happens' do
subject do
monitor.call(worker, job, queue) do
raise Gitlab::SidekiqMonitor::CancelledError
end
end
it 'does skip this job' do
expect { subject }.to raise_error(Sidekiq::JobRetry::Skip)
end
end
end
end

View File

@ -54,7 +54,7 @@ describe Gitlab::SidekiqMonitor do
it 'logs start message' do
expect(Sidekiq.logger).to receive(:info)
.with(
class: described_class,
class: described_class.to_s,
action: 'start',
message: 'Starting Monitor Daemon')
@ -66,7 +66,7 @@ describe Gitlab::SidekiqMonitor do
it 'logs stop message' do
expect(Sidekiq.logger).to receive(:warn)
.with(
class: described_class,
class: described_class.to_s,
action: 'stop',
message: 'Stopping Monitor Daemon')
@ -78,7 +78,7 @@ describe Gitlab::SidekiqMonitor do
it 'logs StandardError message' do
expect(Sidekiq.logger).to receive(:warn)
.with(
class: described_class,
class: described_class.to_s,
action: 'exception',
message: 'My Exception')
@ -91,7 +91,7 @@ describe Gitlab::SidekiqMonitor do
it 'logs and raises Exception message' do
expect(Sidekiq.logger).to receive(:warn)
.with(
class: described_class,
class: described_class.to_s,
action: 'exception',
message: 'My Exception')
@ -131,13 +131,13 @@ describe Gitlab::SidekiqMonitor do
expect(Sidekiq.logger).to receive(:info)
.with(
class: described_class,
class: described_class.to_s,
action: 'start',
message: 'Starting Monitor Daemon')
expect(Sidekiq.logger).to receive(:info)
.with(
class: described_class,
class: described_class.to_s,
channel: described_class::NOTIFICATION_CHANNEL,
message: 'Received payload on channel',
payload: payload
@ -215,7 +215,7 @@ describe Gitlab::SidekiqMonitor do
it 'does log cancellation message' do
expect(Sidekiq.logger).to receive(:warn)
.with(
class: described_class,
class: described_class.to_s,
action: 'cancel',
message: 'Canceling thread with CancelledError',
jid: 'my-jid',