summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzciński <ayufan@ayufan.eu>2019-08-21 12:03:42 +0200
committerKamil Trzciński <ayufan@ayufan.eu>2019-08-21 13:21:55 +0200
commit8d17c4dae6b4662dddffe9e2ddca8100e8cd3d0b (patch)
treef13731d3bdb703e868517403afd83f92d965ea7e
parentcb193cd6b54ee9ac0cf87650100585293540abfa (diff)
downloadgitlab-ce-8d17c4dae6b4662dddffe9e2ddca8100e8cd3d0b.tar.gz
Properly handle `sidekiq` skipsidekiq-interrupt-running-jobs
Transform `CancelledError` into `JobRetry::Skip`
-rw-r--r--config/initializers/sidekiq.rb7
-rw-r--r--lib/gitlab/sidekiq_middleware/monitor.rb3
-rw-r--r--lib/gitlab/sidekiq_monitor.rb12
-rw-r--r--spec/lib/gitlab/sidekiq_middleware/monitor_spec.rb12
-rw-r--r--spec/lib/gitlab/sidekiq_monitor_spec.rb14
5 files changed, 31 insertions, 17 deletions
diff --git a/config/initializers/sidekiq.rb b/config/initializers/sidekiq.rb
index e145af5e2d5..9f3e104bc2b 100644
--- a/config/initializers/sidekiq.rb
+++ b/config/initializers/sidekiq.rb
@@ -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?
diff --git a/lib/gitlab/sidekiq_middleware/monitor.rb b/lib/gitlab/sidekiq_middleware/monitor.rb
index 2d0e5a6d635..0d88fe760d3 100644
--- a/lib/gitlab/sidekiq_middleware/monitor.rb
+++ b/lib/gitlab/sidekiq_middleware/monitor.rb
@@ -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
diff --git a/lib/gitlab/sidekiq_monitor.rb b/lib/gitlab/sidekiq_monitor.rb
index 9ec3be48adb..9842f1f53f7 100644
--- a/lib/gitlab/sidekiq_monitor.rb
+++ b/lib/gitlab/sidekiq_monitor.rb
@@ -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,
diff --git a/spec/lib/gitlab/sidekiq_middleware/monitor_spec.rb b/spec/lib/gitlab/sidekiq_middleware/monitor_spec.rb
index 3ca2ddf3cb1..2933d26a387 100644
--- a/spec/lib/gitlab/sidekiq_middleware/monitor_spec.rb
+++ b/spec/lib/gitlab/sidekiq_middleware/monitor_spec.rb
@@ -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
diff --git a/spec/lib/gitlab/sidekiq_monitor_spec.rb b/spec/lib/gitlab/sidekiq_monitor_spec.rb
index 62baa326887..bbd7bf90217 100644
--- a/spec/lib/gitlab/sidekiq_monitor_spec.rb
+++ b/spec/lib/gitlab/sidekiq_monitor_spec.rb
@@ -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',