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


##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -30,25 +30,19 @@ public class Halt {
 
   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, "FATAL " + msg, null);
   }
 
   public static void halt(final String msg, int status) {
-    halt(status, new Runnable() {
-      @Override
-      public void run() {
-        log.error("FATAL {}", msg);
-      }
-    });
+    halt(status, "FATAL " + msg, null);
   }
 
-  public static void halt(final int status, Runnable runnable) {
+  public static void halt(final int status, String msg, Runnable runnable) {
 
+    // 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(msg);
+    log.error(msg);

Review Comment:
   These should be moved into the try/finally.



##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -30,25 +30,19 @@ public class Halt {
 
   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, "FATAL " + msg, null);
   }
 
   public static void halt(final String msg, int status) {
-    halt(status, new Runnable() {
-      @Override
-      public void run() {
-        log.error("FATAL {}", msg);
-      }
-    });
+    halt(status, "FATAL " + msg, null);
   }
 
-  public static void halt(final int status, Runnable runnable) {
+  public static void halt(final int status, String msg, Runnable runnable) {

Review Comment:
   Seems like the runnable was primary related to logging, wondering if it 
should be removed and we just pass an optional exception instead.  Then this 
code can deal w/ logging the exception if it is not null.
   
   ```suggestion
     public static void halt(final int status, String msg, Exception e) {
   ```



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