You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2025/05/22 13:36:58 UTC
[PR] Changed Halt to print msg to stderr [accumulo]
dlmarion opened a new pull request, #5574:
URL: https://github.com/apache/accumulo/pull/5574
Logging that happens in proximity to a Runtime.halt may not make it to the log before the proceess shuts down if an asynchronous logging implementation is used.
This change modifies the Halt methods to print any log message to stderr and to log.error.
Closes #5545
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2102781882
##########
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:
ServiceLockSupport and TabletClientHandler have other actions in the Runnable
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #5574:
URL: https://github.com/apache/accumulo/pull/5574
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2104570239
##########
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:
I added the Optional and added overloaded methods to Halt to reduce the amount of noise in the calling code.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2102779231
##########
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:
You mean at the beginning of the try block? I don't think it should go into the finally block, that would be too late.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2112007754
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,43 @@ 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) {
Review Comment:
I removed the Optional in e0910de.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#issuecomment-2904401268
@keith-turner @ctubbsii - I believe I hit all of your points in this latest commit. I also went through and looked at all uses of `Halt` and found some places where the exception was not being passed.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2112007236
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,43 @@ 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);
Review Comment:
Nothing was calling that method, so I removed it in e0910de.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2112009518
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,43 @@ 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 {
+ final String errorMessage = "FATAL " + msg;
+
+ Throwable t = null;
+ if (exception.isPresent()) {
+ t = exception.orElseThrow();
+ }
+ // 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(errorMessage);
+ if (t != null) {
Review Comment:
I removed the Optional in e0910de.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2110594726
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,43 @@ 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);
Review Comment:
It's a little weird to me that we have any methods that call this. Do we have cases where we want to fatally halt, but still return with a "0" exit code? I would assume we'd always exit with non-zero if we're halting because of a fatal issue.
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,43 @@ 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);
Review Comment:
It might be simpler to just inline some of these overloaded methods. It's confusing to know when to call which one.
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,43 @@ 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 {
+ final String errorMessage = "FATAL " + msg;
+
+ Throwable t = null;
+ if (exception.isPresent()) {
+ t = exception.orElseThrow();
+ }
+ // 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(errorMessage);
+ if (t != null) {
Review Comment:
It seems odd to do a null check here, after the Optional was already evaluated. I wonder if this can be made more succinct.
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,43 @@ 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) {
Review Comment:
We're mixing Optionals with nullables. Either both params should be nullable non-Optionals or both should use non-null Optionals. We shouldn't mix them.
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,43 @@ 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 {
+ final String errorMessage = "FATAL " + msg;
+
+ Throwable t = null;
+ if (exception.isPresent()) {
+ t = exception.orElseThrow();
+ }
Review Comment:
This is equivalent to:
```suggestion
Throwable t = exception.orElse(null);
```
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2109788080
##########
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.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2112516143
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -28,34 +28,43 @@
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);
- }
- });
+ public static void halt(final int status, final String msg) {
+ halt(status, msg, null, 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 int status, final String msg, final Throwable exception) {
+ halt(status, msg, exception, null);
}
- public static void halt(final int status, Runnable runnable) {
+ public static void halt(final int status, final String msg, final Runnable runnable) {
+ halt(status, msg, null, runnable);
+ }
+ public static void halt(final int status, final String msg, final Throwable exception,
+ final Runnable runnable) {
try {
+
// give ourselves a little time to try and do something
Threads.createThread("Halt Thread", () -> {
sleepUninterruptibly(100, MILLISECONDS);
Runtime.getRuntime().halt(status);
}).start();
+ final String errorMessage = "FATAL " + msg;
+ // 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(errorMessage);
+ if (exception != null) {
+ exception.printStackTrace();
+ }
+ System.err.flush();
+
+ if (exception != null) {
+ log.error(errorMessage, exception);
+ } else {
+ log.error(errorMessage);
Review Comment:
The first parameter for the `log.error()` function is a format string, not a fixed string. So, this could result in errors. It should instead be:
```suggestion
log.error("{}", errorMessage, exception);
} else {
log.error("{}", errorMessage);
```
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2102940477
##########
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:
Fixed in 84d6f26
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2112114397
##########
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:
ok, fair point. I was thinking of the cases where the Runnable was being used for non-logging actions.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2112140502
##########
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:
Implemented in 75cbefa
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2103805467
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLockSupport.java:
##########
@@ -63,9 +63,8 @@ public void lostLock(LockLossReason reason) {
@Override
public void unableToMonitorLockNode(final Exception e) {
// ACCUMULO-3651 Changed level to error and added FATAL to message for slf4j compatibility
- Halt.halt(-1,
- () -> LOG.error("FATAL: No longer able to monitor {} lock node", serviceName, e));
-
+ Halt.halt(-1, "FATAL: No longer able to monitor " + serviceName + " lock node",
+ () -> e.printStackTrace());
Review Comment:
I'm not sure the comment above this makes any sense anymore.
```suggestion
Halt.halt(-1, "FATAL: No longer able to monitor " + serviceName + " lock node",
e::printStackTrace);
```
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -30,26 +30,20 @@ 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) {
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(msg);
Review Comment:
I'm thinking that we should probably flush STDERR. I've seen processes die too quickly for it to get flushed on its own, and the message sent to System.err never appears.
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java:
##########
@@ -1155,11 +1155,11 @@ static void checkPermission(ServerContext context, TabletHostingServer server,
log.trace("Got {} message from user: {}", request, credentials.getPrincipal());
if (server.getLock() != null && server.getLock().wasLockAcquired()
&& !server.getLock().isLocked()) {
- Halt.halt(1, () -> {
- log.info("Tablet server no longer holds lock during checkPermission() : {}, exiting",
- request);
- server.getGcLogger().logGCInfo(server.getConfiguration());
- });
+ Halt.halt(1,
+ "Tablet server no longer holds lock during checkPermission() : " + request + ", exiting",
+ () -> {
+ server.getGcLogger().logGCInfo(server.getConfiguration());
+ });
Review Comment:
```suggestion
() -> server.getGcLogger().logGCInfo(server.getConfiguration()));
```
##########
server/base/src/main/java/org/apache/accumulo/server/util/FileSystemMonitor.java:
##########
@@ -121,8 +121,8 @@ public FileSystemMonitor(final String procFile, long period, AccumuloConfigurati
try {
checkMount(mount);
} catch (final Exception e) {
- Halt.halt(-42,
- () -> log.error("Exception while checking mount points, halting process", e));
+ Halt.halt(-42, "Exception while checking mount points, halting process",
Review Comment:
Negative 42 is a little crazy. I'm not sure where these numbers come from. But, we really shouldn't be using negative values for exit codes. Some environments don't support that. Positive 42 is less crazy (but still strange and arbitrary).
##########
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:
The optional exception might still be a good idea as a separate parameter, so we're not using the Runnable action for simply passing an exception. Since we're still sending it to the logging framework, it's better to pass the exception, rather than omit it from the logged error and rely on the Runnable to print the stack trace This method can print the stack trace, if an exception is passed, and also give it to the log framework.
##########
server/base/src/main/java/org/apache/accumulo/server/util/FileSystemMonitor.java:
##########
@@ -121,8 +121,8 @@ public FileSystemMonitor(final String procFile, long period, AccumuloConfigurati
try {
checkMount(mount);
} catch (final Exception e) {
- Halt.halt(-42,
- () -> log.error("Exception while checking mount points, halting process", e));
+ Halt.halt(-42, "Exception while checking mount points, halting process",
+ () -> e.printStackTrace());
Review Comment:
```suggestion
e::printStackTrace);
```
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLockSupport.java:
##########
@@ -145,16 +144,15 @@ public void lostLock(final LockLossReason reason) {
LOG.warn("{} lost lock (reason = {}), not halting because shutdown is complete.",
serviceName, reason);
} else {
- Halt.halt(1, () -> {
- LOG.error("{} lost lock (reason = {}), exiting.", serviceName, reason);
- lostLockAction.accept(serviceName);
- });
+ Halt.halt(1, serviceName + " lost lock (reason = " + reason + "), exiting.",
+ () -> lostLockAction.accept(serviceName));
}
}
@Override
public void unableToMonitorLockNode(final Exception e) {
- Halt.halt(1, () -> LOG.error("Lost ability to monitor {} lock, exiting.", serviceName, e));
+ Halt.halt(1, "Lost ability to monitor " + serviceName + " lock, exiting.",
+ () -> e.printStackTrace());
Review Comment:
```suggestion
e::printStackTrace);
```
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2109574152
##########
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);
Review Comment:
Could restructure this code to print to stderr, flush stderr, and then log after the flush in case the logging config causes the log to potentially hang.
##########
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:
Could move this code prior to the new logging and prints. This code is trying to defends against something like the logging call or print call hanging.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2109810166
##########
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);
Review Comment:
Reordered code in 0ccebe0 log to stderr, then log to logging framework
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2112009891
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,43 @@ 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 {
+ final String errorMessage = "FATAL " + msg;
+
+ Throwable t = null;
+ if (exception.isPresent()) {
+ t = exception.orElseThrow();
+ }
Review Comment:
I removed the Optional in e0910de.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2112507584
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -29,27 +31,43 @@ 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);
Review Comment:
I think some of the other methods are called and explicitly pass 0, which I still think is weird, but at least it's more explicit, so we can comment there, if there's a reason to use 0, on a case-by-case basis. I wouldn't expect those to all be reviewed in this PR, though. Maybe a follow-on at some point, that also addresses all the negative values.
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Changed Halt to print msg to stderr [accumulo]
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #5574:
URL: https://github.com/apache/accumulo/pull/5574#discussion_r2112529341
##########
core/src/main/java/org/apache/accumulo/core/util/Halt.java:
##########
@@ -28,34 +28,43 @@
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);
- }
- });
+ public static void halt(final int status, final String msg) {
+ halt(status, msg, null, 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 int status, final String msg, final Throwable exception) {
+ halt(status, msg, exception, null);
}
- public static void halt(final int status, Runnable runnable) {
+ public static void halt(final int status, final String msg, final Runnable runnable) {
+ halt(status, msg, null, runnable);
+ }
+ public static void halt(final int status, final String msg, final Throwable exception,
+ final Runnable runnable) {
try {
+
// give ourselves a little time to try and do something
Threads.createThread("Halt Thread", () -> {
sleepUninterruptibly(100, MILLISECONDS);
Runtime.getRuntime().halt(status);
}).start();
+ final String errorMessage = "FATAL " + msg;
+ // 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(errorMessage);
+ if (exception != null) {
+ exception.printStackTrace();
+ }
+ System.err.flush();
+
+ if (exception != null) {
+ log.error(errorMessage, exception);
+ } else {
+ log.error(errorMessage);
Review Comment:
Fixed in 96888ee01fc3894959278f7ac2b7826e40229632
--
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-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org