-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve handling of SIGCONT after SIGTERM #4034
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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_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) | ||||||||||
|
||||||||||
begin | ||||||||||
Process.kill :CONT, $$ | ||||||||||
rescue | ||||||||||
end | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we remove this |
||||||||||
|
||||||||||
sleep 1 | ||||||||||
|
||||||||||
assert_true File.exist?(@sigdump_path) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Power assert style is preferred for better failure message:
Suggested change
|
||||||||||
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) | ||||||||||
|
||||||||||
begin | ||||||||||
Process.kill :TERM, $$ | ||||||||||
Process.kill :CONT, $$ | ||||||||||
rescue | ||||||||||
end | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we remove this |
||||||||||
|
||||||||||
sleep 1 | ||||||||||
|
||||||||||
debug_msg = '[debug]: fluentd main process get SIGTERM' + "\n" | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
logs = $log.out.logs | ||||||||||
assert{ logs.any?{|log| log.include?(debug_msg) } } | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems to be the same code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, sorry. This is garbage. |
||||||||||
|
||||||||||
assert_false File.exist?(@sigdump_path) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
ensure | ||||||||||
$log.out.reset if $log && $log.out && $log.out.respond_to?(:reset) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
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,39 @@ 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 | ||||||||||
|
||||||||||
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? | ||||||||||
|
||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Process.pid
is preferred than$$
for readability.