From e1c39c8669fc9b71d3e4813af6abd75ac3bffbce Mon Sep 17 00:00:00 2001 From: abetomo Date: Tue, 31 Jan 2023 22:37:04 +0900 Subject: [PATCH 1/4] Improve handling of SIGCONT after SIGTERM Signed-off-by: abetomo --- lib/fluent/supervisor.rb | 9 ++++ test/test_supervisor.rb | 88 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 95 insertions(+), 2 deletions(-) diff --git a/lib/fluent/supervisor.rb b/lib/fluent/supervisor.rb index b372f92249..57fc604458 100644 --- a/lib/fluent/supervisor.rb +++ b/lib/fluent/supervisor.rb @@ -190,6 +190,12 @@ def install_supervisor_signal_handlers $log.debug 'fluentd supervisor process got SIGUSR2' supervisor_sigusr2_handler end + + term_handler = trap :TERM do + # After SIGTERM, disable the sigdump file by ignoring SIGCONT. + trap(:CONT, nil) + term_handler.call + end end if Fluent.windows? @@ -922,6 +928,9 @@ def install_main_process_signal_handlers end trap :TERM do + # After SIGTERM, disable the sigdump file by ignoring SIGCONT. + trap(:CONT, nil) + $log.debug "fluentd main process get SIGTERM" unless @finished @finished = true diff --git a/test/test_supervisor.rb b/test/test_supervisor.rb index c8b3f5845b..9fbbb84441 100644 --- a/test/test_supervisor.rb +++ b/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-#{$$}.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 @@ -213,6 +214,52 @@ def test_main_process_signal_handlers $log.out.reset if $log && $log.out && $log.out.respond_to?(:reset) end + def test_cont_in_main_processsignal_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) + + begin + Process.kill :CONT, $$ + rescue + end + + sleep 1 + + assert_true File.exist?(@sigdump_path) + ensure + File.delete(@sigdump_path) if File.exist?(@sigdump_path) + end + + def test_term_cont_in_main_processsignal_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) + + begin + Process.kill :TERM, $$ + Process.kill :CONT, $$ + rescue + end + + sleep 1 + + debug_msg = '[debug]: fluentd main process get SIGTERM' + "\n" + logs = $log.out.logs + assert{ logs.any?{|log| log.include?(debug_msg) } } + + assert_false File.exist?(@sigdump_path) + ensure + $log.out.reset if $log && $log.out && $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? @@ -239,7 +286,7 @@ def test_main_process_command_handlers $log.out.reset if $log && $log.out && $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 @@ -260,6 +307,43 @@ def test_supervisor_signal_handler $log.out.reset if $log && $log.out && $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 + begin + Process.kill :CONT, $$ + rescue + end + + sleep 1 + + assert_true File.exist?(@sigdump_path) + ensure + File.delete(@sigdump_path) if File.exist?(@sigdump_path) + end + + # TODO Sending SIGTERM terminates the test itself. + # I think we have to figure out a good way. + # def test_term_cont_in_supervisor_signal_handler + # omit "Windows cannot handle signals" if Fluent.windows? + # + # server = DummyServer.new + # server.install_supervisor_signal_handlers + # begin + # Process.kill :TERM, $$ + # Process.kill :CONT, $$ + # rescue + # end + # + # sleep 1 + # + # 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? From 9f7fa188dc068a4c4cb44371170472c8cbd0beaa Mon Sep 17 00:00:00 2001 From: abetomo Date: Fri, 3 Feb 2023 08:44:56 +0900 Subject: [PATCH 2/4] Check that `term_handler` is a `Proc` Signed-off-by: abetomo --- lib/fluent/supervisor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fluent/supervisor.rb b/lib/fluent/supervisor.rb index 57fc604458..740405a12d 100644 --- a/lib/fluent/supervisor.rb +++ b/lib/fluent/supervisor.rb @@ -194,7 +194,7 @@ def install_supervisor_signal_handlers term_handler = trap :TERM do # After SIGTERM, disable the sigdump file by ignoring SIGCONT. trap(:CONT, nil) - term_handler.call + term_handler.call if term_handler.is_a?(Proc) end end From 01eba25592ea1162a4b9ce3f3dddf3091f7a8acb Mon Sep 17 00:00:00 2001 From: abetomo Date: Fri, 3 Feb 2023 08:46:55 +0900 Subject: [PATCH 3/4] Rename test methods (fix typo) ..._main_processsignal_handlers -> ..._main_process_signal_handlers Signed-off-by: abetomo --- test/test_supervisor.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_supervisor.rb b/test/test_supervisor.rb index 9fbbb84441..7e23d5f4db 100644 --- a/test/test_supervisor.rb +++ b/test/test_supervisor.rb @@ -214,7 +214,7 @@ def test_usr1_in_main_process_signal_handlers $log.out.reset if $log && $log.out && $log.out.respond_to?(:reset) end - def test_cont_in_main_processsignal_handlers + def test_cont_in_main_process_signal_handlers omit "Windows cannot handle signals" if Fluent.windows? opts = Fluent::Supervisor.default_options @@ -233,7 +233,7 @@ def test_cont_in_main_processsignal_handlers File.delete(@sigdump_path) if File.exist?(@sigdump_path) end - def test_term_cont_in_main_processsignal_handlers + def test_term_cont_in_main_process_signal_handlers omit "Windows cannot handle signals" if Fluent.windows? create_debug_dummy_logger From 88827b10240a0317c2a96ef67fa5fd699859ca58 Mon Sep 17 00:00:00 2001 From: abetomo Date: Fri, 3 Feb 2023 22:15:49 +0900 Subject: [PATCH 4/4] test: add test_term_cont_in_supervisor_signal_handler Signed-off-by: abetomo --- test/test_supervisor.rb | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/test/test_supervisor.rb b/test/test_supervisor.rb index 7e23d5f4db..cd1e4d528c 100644 --- a/test/test_supervisor.rb +++ b/test/test_supervisor.rb @@ -324,25 +324,21 @@ def test_cont_in_supervisor_signal_handler File.delete(@sigdump_path) if File.exist?(@sigdump_path) end - # TODO Sending SIGTERM terminates the test itself. - # I think we have to figure out a good way. - # def test_term_cont_in_supervisor_signal_handler - # omit "Windows cannot handle signals" if Fluent.windows? - # - # server = DummyServer.new - # server.install_supervisor_signal_handlers - # begin - # Process.kill :TERM, $$ - # Process.kill :CONT, $$ - # rescue - # end - # - # sleep 1 - # - # assert_not 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 + begin + Process.kill :TERM, $$ + Process.kill :CONT, $$ + rescue + end + + assert_false 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?