Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STORM-1600: Do not report exceptions after jvm shutdown #1575

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 11 additions & 9 deletions storm-core/src/clj/org/apache/storm/daemon/executor.clj
Expand Up @@ -205,15 +205,17 @@
:report-error-and-die (reify
Thread$UncaughtExceptionHandler
(uncaughtException [this _ error]
(try
((:report-error <>) error)
(catch Exception e
(log-error e "Error while reporting error to cluster, proceeding with shutdown")))
(if (or
(Utils/exceptionCauseIsInstanceOf InterruptedException error)
(Utils/exceptionCauseIsInstanceOf java.io.InterruptedIOException error))
(log-message "Got interrupted excpetion shutting thread down...")
((:suicide-fn <>)))))
(if (Utils/isShutdownUnderProgress)
(log-warn error "Uncaught exception in thread after jvm shutdown")
((try
((:report-error <>) error)
(catch Exception e
(log-error e "Error while reporting error to cluster, proceeding with shutdown")))
(if (or
(Utils/exceptionCauseIsInstanceOf InterruptedException error)
(Utils/exceptionCauseIsInstanceOf java.io.InterruptedIOException error))
(log-message "Got interrupted excpetion shutting thread down...")
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: fix spelling of exception.

((:suicide-fn <>)))))))
:sampler (ConfigUtils/mkStatsSampler storm-conf)
:backpressure (atom false)
:spout-throttling-metrics (if (= (keyword executor-type) :spout)
Expand Down
12 changes: 12 additions & 0 deletions storm-core/src/jvm/org/apache/storm/utils/Utils.java
Expand Up @@ -123,6 +123,7 @@
import java.util.TreeMap;
import java.util.UUID;
import java.util.concurrent.Callable;
import java.util.concurrent.atomic.AtomicReference;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.regex.Matcher;
Expand All @@ -139,6 +140,9 @@ public class Utils {
// tests by subclassing.
private static Utils _instance = new Utils();

// A static flag to indicate if jvm shutdown is in progress
private static AtomicReference<Boolean> isJVMShutdownInitiated = new AtomicReference<>(Boolean.FALSE);

/**
* Provide an instance of this class for delegates to use. To mock out
* delegated methods, provide an instance of a subclass that overrides the
Expand Down Expand Up @@ -1202,6 +1206,13 @@ public static boolean exceptionCauseIsInstanceOf(Class klass, Throwable throwabl
return false;
}

/**
* This method can detect if jvm shutdown has been initiated in response to an uncaught exception in executor
*/
public static boolean isShutdownUnderProgress() {
return isJVMShutdownInitiated.get();
}

/**
* Is the cluster configured to interact with ZooKeeper in a secure way?
* This only works when called from within Nimbus or a Supervisor process.
Expand Down Expand Up @@ -1772,6 +1783,7 @@ public static String uuid() {
public static void exitProcess (int val, String msg) {
String combinedErrorMessage = "Halting process: " + msg;
LOG.error(combinedErrorMessage, new RuntimeException(combinedErrorMessage));
isJVMShutdownInitiated.set(Boolean.TRUE);
Copy link
Contributor

Choose a reason for hiding this comment

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

ShellSpout and ShellBolt just call System.exit() directly from their die() method. You may want to change those too.

Runtime.getRuntime().exit(val);
}

Expand Down