keith-turner commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2112095724


##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,35 @@ public class Halt {
   private static final Logger log = LoggerFactory.getLogger(Halt.class);
 
   public static void halt(final String msg) {
-    // ACCUMULO-3651 Changed level to error and added FATAL to message for 
slf4j compatibility
-    halt(0, new Runnable() {
-      @Override
-      public void run() {
-        log.error("FATAL {}", msg);
-      }
-    });
+    halt(0, msg, Optional.empty(), null);
   }
 
-  public static void halt(final String msg, int status) {
-    halt(status, new Runnable() {
-      @Override
-      public void run() {
-        log.error("FATAL {}", msg);
-      }
-    });
+  public static void halt(final String msg, final int status) {
+    halt(status, msg, Optional.empty(), null);
   }
 
-  public static void halt(final int status, Runnable runnable) {
+  public static void halt(final int status, final String msg, final 
Optional<Throwable> exception) {
+    halt(status, msg, exception, null);
+  }
 
+  public static void halt(final int status, final String msg, final Runnable 
runnable) {
+    halt(status, msg, Optional.empty(), runnable);
+  }
+
+  public static void halt(final int status, final String msg, final 
Optional<Throwable> exception,
+      final Runnable runnable) {
     try {
+      // Printing to stderr and to the log in case the message does not make
+      // it to the log. This could happen if an asynchronous logging impl is 
used
+      System.err.println("FATAL " + msg);
+      if (exception.isPresent()) {
+        Throwable t = exception.orElseThrow();
+        t.printStackTrace();
+        log.error("FATAL " + msg, t);
+      } else {
+        log.error("FATAL " + msg);
+      }
+      System.err.flush();
       // give ourselves a little time to try and do something
       Threads.createThread("Halt Thread", () -> {

Review Comment:
   > I'm not sure about that. I thought the thread was to limit the amount of 
time that the Runnable can run. That's what the comment seems to imply anyway.
   
   Right and the runnable used to log in a lot of cases.  In the original code 
the thread was started first thing before running the runnable.  That way if 
the logging or whatever else the runnable was doing got stuck, then the halt 
would still happen.  Now the halt method is doing some logging itself, it 
coiuld still start the thread before doing this in case the logging gets stuck.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to