diff --git a/lib/good_job.rb b/lib/good_job.rb index c0389f7e8..aa2d26dfe 100644 --- a/lib/good_job.rb +++ b/lib/good_job.rb @@ -126,13 +126,13 @@ def self.restart(timeout: -1) _shutdown_all(_executables, :restart, timeout: timeout) end - # Sends +#shutdown+ or +#restart+ to executable objects ({GoodJob::Notifier}, {GoodJob::Poller}, {GoodJob::Scheduler}) + # Sends +#shutdown+ or +#restart+ to executable objects ({GoodJob::Notifier}, {GoodJob::Poller}, {GoodJob::Scheduler}, {GoodJob::MultiScheduler}) # @param executables [Array] Objects to shut down. # @param method_name [:symbol] Method to call, e.g. +:shutdown+ or +:restart+. # @param timeout [nil,Numeric] # @return [void] def self._shutdown_all(executables, method_name = :shutdown, timeout: -1) - if timeout.positive? + if timeout.is_a?(Numeric) && timeout.positive? executables.each { |executable| executable.send(method_name, timeout: nil) } stop_at = Time.current + timeout diff --git a/lib/good_job/adapter.rb b/lib/good_job/adapter.rb index 241688e9d..949080106 100644 --- a/lib/good_job/adapter.rb +++ b/lib/good_job/adapter.rb @@ -123,7 +123,7 @@ def shutdown(timeout: :default, wait: nil) timeout end - executables = [@notifier, @poller, @scheduler].compact + executables = [@notifier, @poller, @scheduler, @cron_manager].compact GoodJob._shutdown_all(executables, timeout: timeout) end diff --git a/lib/good_job/cli.rb b/lib/good_job/cli.rb index f7b49c11f..2248921fc 100644 --- a/lib/good_job/cli.rb +++ b/lib/good_job/cli.rb @@ -91,7 +91,7 @@ def start scheduler = GoodJob::Scheduler.from_configuration(configuration, warm_cache_on_initialize: true) notifier.recipients << [scheduler, :create_thread] poller.recipients << [scheduler, :create_thread] - cron_manager = GoodJob::CronManager.new(configuration.cron, start_on_initialize: true) if configuration.enable_cron? + _cron_manager = GoodJob::CronManager.new(configuration.cron, start_on_initialize: true) if configuration.enable_cron? @stop_good_job_executable = false %w[INT TERM].each do |signal| trap(signal) { @stop_good_job_executable = true } @@ -102,8 +102,7 @@ def start break if @stop_good_job_executable || scheduler.shutdown? || notifier.shutdown? end - executors = [notifier, poller, cron_manager, scheduler].compact - GoodJob._shutdown_all(executors, timeout: configuration.shutdown_timeout) + GoodJob.shutdown(timeout: configuration.shutdown_timeout) end default_task :start diff --git a/spec/lib/good_job/cli_spec.rb b/spec/lib/good_job/cli_spec.rb index bfaa484ad..ca8b79130 100644 --- a/spec/lib/good_job/cli_spec.rb +++ b/spec/lib/good_job/cli_spec.rb @@ -9,6 +9,7 @@ before do stub_const 'GoodJob::CLI::RAILS_ENVIRONMENT_RB', File.expand_path("spec/test_app/config/environment.rb") allow(GoodJob::Scheduler).to receive(:new).and_return scheduler_mock + allow(GoodJob).to receive(:shutdown).and_call_original end describe '#start' do @@ -32,7 +33,7 @@ sleep_until { cli_thread.fulfilled? } expect(GoodJob::Scheduler).to have_received(:new) - expect(scheduler_mock).to have_received(:shutdown) + expect(GoodJob).to have_received(:shutdown) end describe 'max threads' do diff --git a/spec/lib/good_job_spec.rb b/spec/lib/good_job_spec.rb index d331e367a..82a26e37f 100644 --- a/spec/lib/good_job_spec.rb +++ b/spec/lib/good_job_spec.rb @@ -2,8 +2,9 @@ require 'rails_helper' describe GoodJob do - let(:configuration) { GoodJob::Configuration.new({ queues: 'mice:1', poll_interval: -1 }) } - let!(:scheduler) { GoodJob::Scheduler.from_configuration(configuration) } + let(:configuration) { GoodJob::Configuration.new({ queues: 'mice:1;elephants:1', poll_interval: -1 }) } + let!(:multi_scheduler) { GoodJob::Scheduler.from_configuration(configuration) } + let(:scheduler) { multi_scheduler.schedulers.first } let!(:notifier) { GoodJob::Notifier.new([scheduler, :create_thread]) } describe '.shutdown' do @@ -13,6 +14,13 @@ expect(scheduler.shutdown?).to eq true expect(notifier.shutdown?).to eq true end + + it 'shuts down when there is a timeout set' do + described_class.shutdown(timeout: 5) + + expect(scheduler.shutdown?).to eq true + expect(notifier.shutdown?).to eq true + end end describe '.shutdown?' do @@ -27,9 +35,16 @@ it 'restarts down all scheduler and notifier instances' do described_class.shutdown + expect(scheduler.shutdown?).to eq true + expect(notifier.shutdown?).to eq true + end + end + + describe '._shutdown_all' do + it 'delegates shutdown methods to multiple instances' do expect do - described_class.restart - end.to change(described_class, :shutdown?).from(true).to(false) + described_class._shutdown_all([multi_scheduler, notifier], timeout: 5) + end.to change(described_class, :shutdown?).from(false).to(true) end end