[GitHub] [spark] HeartSaVioR commented on a change in pull request #27832: [SPARK-31011][CORE] Log better message if SIGPWR is not supported while setting up decommission

[GitBox <gi...@apache.org>]

HeartSaVioR commented on a change in pull request #27832: [SPARK-31011][CORE] Log better message if SIGPWR is not supported while setting up decommission
URL: https://github.com/apache/spark/pull/27832#discussion_r390700925
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/util/SignalUtils.scala
 ##########
 @@ -58,18 +58,45 @@ private[spark] object SignalUtils extends Logging {
    */
   def register(signal: String)(action: => Boolean): Unit = synchronized {
     if (SystemUtils.IS_OS_UNIX) {
-      try {
-        val handler = handlers.getOrElseUpdate(signal, {
-          logInfo("Registering signal handler for " + signal)
-          new ActionHandler(new Signal(signal))
-        })
-        handler.register(action)
-      } catch {
-        case ex: Exception => logWarning(s"Failed to register signal handler for " + signal, ex)
-      }
+      register(signal, s"Failed to register signal handler for " + signal,
+        logStackTrace = true)(action)
     }
   }
 
+  /**
+   * Adds an action to be run when a given signal is received by this process.
+   *
+   * This method receives failMessage as additional parameter, which would be logged when it fails
+   * to register the signal. Here the failures include the cases 1) OS doesn't support signal at
+   * all 2) OS doesn't support given signal (Could be possible with non-POSIX signals)
+   *
+   * All actions for a given signal are run in a separate thread.
+   */
+  def register(
+      signal: String,
+      failMessage: => String,
+      logStackTrace: Boolean = true)(
+      action: => Boolean): Unit = synchronized {
+    try {
+      registerSignal(signal)(action)
+    } catch {
+      case ex: Exception =>
+        if (logStackTrace) {
+          logWarning(failMessage, ex)
+        } else {
+          logWarning(failMessage)
+        }
+    }
+  }
+
+  private def registerSignal(signal: String)(action: => Boolean): Unit = synchronized {
 
 Review comment:
   Ah yes that was added before cleaning up two methods, and now it is being called only once. Will change.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org