Skip to content

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

  1. Move SignalLogger to hdds-server-framework from hdds-common, as it is only used by servers, not clients.
  2. Replace usage of sun.misc.Signal with jnr-posix.

Key detail missed in previous attempt (#6876) is to invoke the default signal handler (which exits the process). jnr-posix currently does not return the default handler, so we need to explicitly call System.exit as a workaround.

https://issues.apache.org/jira/browse/HDDS-11078

How was this patch tested?

Tested with real process on Linux:

$ cd hadoop-ozone/dist/target/ozone-1.5.0-SNAPSHOT
$ bin/ozone s3g
...
[main] INFO s3.Gateway: registered UNIX signal handlers for [SIGHUP, SIGINT, SIGTERM]
...
[main] INFO http.BaseHttpServer: HTTP server of s3gateway listening at http://0.0.0.0:9878

Sent signal from other terminal:

$ kill -SIGTERM $(ps x | grep 'proc_s3[g]' | awk '{ print $1 }')

[SIGTERM handler] ERROR s3.Gateway: RECEIVED SIGNAL 15: SIGTERM
[shutdown-hook-0] INFO s3.Gateway: SHUTDOWN_MSG: 
/************************************************************
SHUTDOWN_MSG: Shutting down Gateway at ...
************************************************************/
[shutdown-hook-0] INFO s3.Gateway: Stopping Ozone S3 gateway
...
[JvmPauseMonitor0] INFO util.JvmPauseMonitor: JvmPauseMonitor-S3G: Stopped

Also with other signals (after restart):

$ kill -SIGINT $(ps x | grep 'proc_s3[g]' | awk '{ print $1 }')

[SIGINT handler] ERROR s3.Gateway: RECEIVED SIGNAL 2: SIGINT
(same output)
$ kill -SIGHUP $(ps x | grep 'proc_s3[g]' | awk '{ print $1 }')

[SIGHUP handler] ERROR s3.Gateway: RECEIVED SIGNAL 1: SIGHUP
(same output)

@adoroszlai adoroszlai self-assigned this Jul 30, 2024
@adoroszlai adoroszlai requested a review from hemantk-12 July 30, 2024 17:19
Copy link
Contributor

@hemantk-12 hemantk-12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @adoroszlai for the quick fix.

LGTM.

Comment on lines +61 to +62
SignalHandler handler = POSIX_IMPL.signal(signal, this);
prevHandler = handler != null ? handler : DEFAULT_HANDLER;
Copy link
Contributor

@hemantk-12 hemantk-12 Jul 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the change missed in the previous PR #6876.

@adoroszlai adoroszlai merged commit cc95ee3 into apache:master Aug 1, 2024
@adoroszlai adoroszlai deleted the HDDS-11078 branch August 1, 2024 07:09
@adoroszlai
Copy link
Contributor Author

Thanks @hemantk-12 for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants