From 44cf3cf975a6eee051af0003ce2c1873e4516ece Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 29 Aug 2018 12:36:59 +1000 Subject: [PATCH] FIX: queue heartbeats in readonly modes If sidekiq is paused or Discourse is in readonly continue to queue heartbeats If we do not do that then a master process can end up reaping sidekiq workers and causing various badness This also impacts restore which can do weird stuff TM in cases like this --- app/jobs/base.rb | 5 +++-- lib/sidekiq/pausable.rb | 2 +- spec/components/sidekiq/pausable_spec.rb | 25 ++++++++++++++++++++++++ spec/jobs/heartbeat_spec.rb | 19 ++++++++++++++++++ 4 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 spec/jobs/heartbeat_spec.rb diff --git a/app/jobs/base.rb b/app/jobs/base.rb index 7e9853b391d..3747969fbf2 100644 --- a/app/jobs/base.rb +++ b/app/jobs/base.rb @@ -183,8 +183,9 @@ module Jobs extend MiniScheduler::Schedule def perform(*args) - return if Discourse.readonly_mode? - super + if (Jobs::Heartbeat === self) || !Discourse.readonly_mode? + super + end end end diff --git a/lib/sidekiq/pausable.rb b/lib/sidekiq/pausable.rb index 8f4d3da28de..49ca1eb6f66 100644 --- a/lib/sidekiq/pausable.rb +++ b/lib/sidekiq/pausable.rb @@ -74,7 +74,7 @@ class Sidekiq::Pausable end def call(worker, msg, queue) - if Sidekiq.paused? + if Sidekiq.paused? && !(Jobs::RunHeartbeat === worker) worker.class.perform_in(@delay, *msg['args']) else start = Process.clock_gettime(Process::CLOCK_MONOTONIC) diff --git a/spec/components/sidekiq/pausable_spec.rb b/spec/components/sidekiq/pausable_spec.rb index 092b3190013..7a9f86c8c1b 100644 --- a/spec/components/sidekiq/pausable_spec.rb +++ b/spec/components/sidekiq/pausable_spec.rb @@ -2,10 +2,35 @@ require 'rails_helper' require_dependency 'sidekiq/pausable' describe Sidekiq do + after do + Sidekiq.unpause! + end + it "can pause and unpause" do Sidekiq.pause! expect(Sidekiq.paused?).to eq(true) Sidekiq.unpause! expect(Sidekiq.paused?).to eq(false) end + + it "can still run heartbeats when paused" do + Sidekiq.pause! + + freeze_time 1.week.from_now + + jobs = Sidekiq::ScheduledSet.new + + Sidekiq::Testing.disable! do + jobs.clear + + middleware = Sidekiq::Pausable.new + middleware.call(Jobs::RunHeartbeat.new, { "args" => [{}] }, "critical") do + "done" + end + + jobs = Sidekiq::ScheduledSet.new + expect(jobs.size).to eq(0) + end + + end end diff --git a/spec/jobs/heartbeat_spec.rb b/spec/jobs/heartbeat_spec.rb new file mode 100644 index 00000000000..f9d90380ddb --- /dev/null +++ b/spec/jobs/heartbeat_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' +require_dependency 'jobs/base' + +describe Jobs::Heartbeat do + after do + Discourse.disable_readonly_mode + end + + it "still enqueues heartbeats in readonly mode" do + freeze_time 1.week.from_now + + Discourse.enable_readonly_mode + + Sidekiq::Testing.inline! do + Jobs::Heartbeat.new.perform(nil) + expect(Jobs::RunHeartbeat.last_heartbeat).to eq(Time.new.to_i) + end + end +end