From a4504ace389ebd4e5e5f0b94df206ce6796173b3 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Tue, 23 May 2017 14:26:17 -0400 Subject: [PATCH 1/3] Reraise SignalExceptions raised in MiqServer#monitor This will let them be properly handled, rather than attempting a reconnect. --- app/models/miq_server.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/miq_server.rb b/app/models/miq_server.rb index 5b453ebed69..3e73d20fad9 100644 --- a/app/models/miq_server.rb +++ b/app/models/miq_server.rb @@ -345,7 +345,7 @@ def monitor Benchmark.realtime_block(:log_active_servers) { log_active_servers } if threshold_exceeded?(:server_log_frequency, now) Benchmark.realtime_block(:worker_monitor) { monitor_workers } if threshold_exceeded?(:worker_monitor_frequency, now) Benchmark.realtime_block(:worker_dequeue) { populate_queue_messages } if threshold_exceeded?(:worker_dequeue_frequency, now) - rescue SystemExit + rescue SystemExit, SignalException # TODO: We're rescuing Exception below. WHY? :bomb: # A SystemExit would be caught below, so we need to explicitly rescue/raise. raise From 37b07612a68e6c60da7102a2e5cb98e764a8e12e Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Fri, 19 May 2017 14:30:17 -0400 Subject: [PATCH 2/3] Move signal handling into the MiqServer object This insures the workers and workers_lock instance variables are still accessible when trying to communicate messages to the workers for a clean shut down. Previously the process survived, but the MiqServer instance was no longer in scope. This caused the workers and workers_lock objects used for message passing via drb to be nil, ensuring that the workers never got the message to exit. --- app/models/miq_server.rb | 7 +++++++ lib/workers/evm_server.rb | 43 --------------------------------------- 2 files changed, 7 insertions(+), 43 deletions(-) diff --git a/app/models/miq_server.rb b/app/models/miq_server.rb index 3e73d20fad9..3f080981c9e 100644 --- a/app/models/miq_server.rb +++ b/app/models/miq_server.rb @@ -369,6 +369,13 @@ def monitor_loop _log.info "Server Monitoring Complete - Timings: #{timings.inspect}" unless timings[:total_time] < server_log_timings_threshold sleep monitor_poll end + rescue Interrupt => e + _log.info("Received #{e.message} signal, killing server") + self.class.kill + exit 1 + rescue SignalException => e + _log.info("Received #{e.message} signal, shutting down server") + shutdown_and_exit end def stop(sync = false) diff --git a/lib/workers/evm_server.rb b/lib/workers/evm_server.rb index b7c476163c5..5bd46095deb 100644 --- a/lib/workers/evm_server.rb +++ b/lib/workers/evm_server.rb @@ -2,8 +2,6 @@ require 'pid_file' class EvmServer - SOFT_INTERRUPT_SIGNALS = ["SIGTERM", "SIGUSR1", "SIGUSR2"] - OPTIONS_PARSER_SETTINGS = [ [:mode, 'EVM Server Mode', String], ] @@ -18,42 +16,6 @@ def initialize(cfg = {}) $log ||= Rails.logger end - def process_hard_signal(s) - exit_code = 1 - message = "Interrupt signal (#{s}) received." - begin - safe_log(message, exit_code) - MiqServer.kill - ensure - do_exit(message, exit_code) - end - end - - def process_soft_signal(s) - MiqServer.stop - ensure - do_exit("Interrupt signal (#{s}) received.", 0) - end - - def do_exit(message = nil, exit_code = 0) - safe_log("#{message} Server exiting.", exit_code) - exit exit_code - end - - def safe_log(message = nil, exit_code = 0) - meth = (exit_code == 0) ? :info : :error - - prefix = "MIQ(EvmServer) " - pid = "PID [#{Process.pid}] " rescue "" - logmsg = "#{prefix}#{pid}#{message}" - - begin - $log.send(meth, logmsg) - rescue - puts "#{meth.to_s.upcase}: #{logmsg}" rescue nil - end - end - def start if pid = MiqServer.running? $log.warn("EVM is already running (PID=#{pid})") @@ -63,11 +25,6 @@ def start PidFile.create(MiqServer.pidfile) set_process_title MiqServer.start - rescue Interrupt => e - process_hard_signal(e.message) - rescue SignalException => e - raise unless SOFT_INTERRUPT_SIGNALS.include?(e.message) - process_soft_signal(e.message) end ## From 994417638d916756d31c96e1bc3a31b9ca811a5f Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Tue, 23 May 2017 14:56:50 -0400 Subject: [PATCH 3/3] Move signal handling specs from evm_server_spec.rb to miq_server_spec.rb --- spec/lib/workers/evm_server_spec.rb | 31 ----------------------------- spec/models/miq_server_spec.rb | 17 ++++++++++++++++ 2 files changed, 17 insertions(+), 31 deletions(-) delete mode 100644 spec/lib/workers/evm_server_spec.rb diff --git a/spec/lib/workers/evm_server_spec.rb b/spec/lib/workers/evm_server_spec.rb deleted file mode 100644 index 25b173fb542..00000000000 --- a/spec/lib/workers/evm_server_spec.rb +++ /dev/null @@ -1,31 +0,0 @@ -require 'workers/evm_server' - -describe EvmServer do - context "#start" do - let(:server) { described_class.new } - - before do - allow(MiqServer).to receive_messages(:running? => false) - allow(server).to receive(:set_database_application_name) - allow(server).to receive(:set_process_title) - allow(PidFile).to receive(:create) - end - - it "SIGINT" do - allow(MiqServer).to receive(:start).and_raise(Interrupt) - expect(server).to receive(:process_hard_signal) - server.start - end - - it "SIGTERM" do - allow(MiqServer).to receive(:start).and_raise(SignalException, "SIGTERM") - expect(server).to receive(:process_soft_signal) - server.start - end - - it "unhandled signal SIGALRM" do - allow(MiqServer).to receive(:start).and_raise(SignalException, "SIGALRM") - expect { server.start }.to raise_error(SignalException, "SIGALRM") - end - end -end diff --git a/spec/models/miq_server_spec.rb b/spec/models/miq_server_spec.rb index d20292941b4..d90b2d07190 100644 --- a/spec/models/miq_server_spec.rb +++ b/spec/models/miq_server_spec.rb @@ -46,6 +46,23 @@ @guid, @miq_server, @zone = EvmSpecHelper.create_guid_miq_server_zone end + describe "#monitor_loop" do + it "calls shutdown_and_exit if SIGTERM is raised" do + expect(@miq_server).to receive(:monitor).and_raise(SignalException, "SIGTERM") + expect(@miq_server).to receive(:shutdown_and_exit) + + @miq_server.monitor_loop + end + + it "kills the server and exits if SIGINT is raised" do + expect(@miq_server).to receive(:monitor).and_raise(Interrupt) + expect(MiqServer).to receive(:kill) + expect(@miq_server).to receive(:exit).with(1) + + @miq_server.monitor_loop + end + end + it "should have proper guid" do expect(@miq_server.guid).to eq(@guid) end