Merge pull request #4043 from abetomo/feature/disable_sigdump_after_stop

Disable sigdump after stop

lib/fluent/supervisor.rb

@@ -355,6 +355,10 @@ def supervisor_get_dump_config_handler
       { conf: @fluentd_conf }
     end
 
+    def dump
+      super unless @stop
+    end
+
     private
 
     def reopen_log
@@ -413,6 +417,10 @@ def spawn(process_manager)
     def after_start
       (config[:worker_pid] ||= {})[@worker_id] = @pm.pid
     end
+
+    def dump
+      super unless @stop
+    end
   end
 
   class Supervisor
@@ -754,12 +762,6 @@ def options
     end
 
     def run_worker
-      begin
-        require 'sigdump/setup'
-      rescue Exception
-        # ignore LoadError and others (related with signals): it may raise these errors in Windows
-      end
-
       Process.setproctitle("worker:#{@system_config.process_name}") if @process_name
 
       if @standalone_worker && @system_config.workers != 1
@@ -940,6 +942,10 @@ def install_main_process_signal_handlers
         trap :USR2 do
           reload_config
         end
+
+        trap :CONT do
+          dump_non_windows
+        end
       end
     end
 
@@ -969,7 +975,7 @@ def install_main_process_command_handlers
             reload_config
           when "DUMP"
             $log.debug "fluentd main process get #{cmd} command"
-            dump
+            dump_windows
           else
             $log.warn "fluentd main process get unknown command [#{cmd}]"
           end
@@ -1020,7 +1026,15 @@ def reload_config
       end
     end
 
-    def dump
+    def dump_non_windows
+      begin
+        Sigdump.dump unless @finished
+      rescue => e
+        $log.error("failed to dump: #{e}")
+      end
+    end
+
+    def dump_windows
       Thread.new do
         begin
           FluentSigdump.dump_windows

test/test_supervisor.rb

@@ -32,6 +32,7 @@ def setup
     @tmp_dir = tmp_dir
     @tmp_root_dir = File.join(@tmp_dir, 'root')
     FileUtils.mkdir_p(@tmp_dir)
+    @sigdump_path = "/tmp/sigdump-#{Process.pid}.log"
   end
 
   def teardown
@@ -191,7 +192,7 @@ def test_system_config
     end
   end
 
-  def test_main_process_signal_handlers
+  def test_usr1_in_main_process_signal_handlers
     omit "Windows cannot handle signals" if Fluent.windows?
 
     create_info_dummy_logger
@@ -210,6 +211,46 @@ def test_main_process_signal_handlers
     $log.out.reset if $log&.out&.respond_to?(:reset)
   end
 
+  def test_cont_in_main_process_signal_handlers
+    omit "Windows cannot handle signals" if Fluent.windows?
+
+    opts = Fluent::Supervisor.default_options
+    sv = Fluent::Supervisor.new(opts)
+    sv.send(:install_main_process_signal_handlers)
+
+    Process.kill :CONT, Process.pid
+
+    sleep 1
+
+    assert{ File.exist?(@sigdump_path) }
+  ensure
+    File.delete(@sigdump_path) if File.exist?(@sigdump_path)
+  end
+
+  def test_term_cont_in_main_process_signal_handlers
+    omit "Windows cannot handle signals" if Fluent.windows?
+
+    create_debug_dummy_logger
+
+    opts = Fluent::Supervisor.default_options
+    sv = Fluent::Supervisor.new(opts)
+    sv.send(:install_main_process_signal_handlers)
+
+    Process.kill :TERM, Process.pid
+    Process.kill :CONT, Process.pid
+
+    sleep 1
+
+    debug_msg = "[debug]: fluentd main process get SIGTERM\n"
+    logs = $log.out.logs
+    assert{ logs.any?{|log| log.include?(debug_msg) } }
+
+    assert{ not File.exist?(@sigdump_path) }
+  ensure
+    $log.out.reset if $log&.out&.respond_to?(:reset)
+    File.delete(@sigdump_path) if File.exist?(@sigdump_path)
+  end
+
   def test_main_process_command_handlers
     omit "Only for Windows, alternative to UNIX signals" unless Fluent.windows?
 
@@ -236,7 +277,7 @@ def test_main_process_command_handlers
     $log.out.reset if $log&.out&.respond_to?(:reset)
   end
 
-  def test_supervisor_signal_handler
+  def test_usr1_in_supervisor_signal_handler
     omit "Windows cannot handle signals" if Fluent.windows?
 
     create_debug_dummy_logger
@@ -255,6 +296,35 @@ def test_supervisor_signal_handler
     $log.out.reset if $log&.out&.respond_to?(:reset)
   end
 
+  def test_cont_in_supervisor_signal_handler
+    omit "Windows cannot handle signals" if Fluent.windows?
+
+    server = DummyServer.new
+    server.install_supervisor_signal_handlers
+
+    Process.kill :CONT, Process.pid
+
+    sleep 1
+
+    assert{ File.exist?(@sigdump_path) }
+  ensure
+    File.delete(@sigdump_path) if File.exist?(@sigdump_path)
+  end
+
+  def test_term_cont_in_supervisor_signal_handler
+    omit "Windows cannot handle signals" if Fluent.windows?
+
+    server = DummyServer.new
+    server.install_supervisor_signal_handlers
+
+    Process.kill :TERM, Process.pid
+    Process.kill :CONT, Process.pid
+
+    assert{ not File.exist?(@sigdump_path) }
+  ensure
+    File.delete(@sigdump_path) if File.exist?(@sigdump_path)
+  end
+
   def test_windows_shutdown_event
     omit "Only for Windows platform" unless Fluent.windows?