CASSANDRA-20974: Heap dump should not be generated on handled exceptions [trunk]

CHANGES.txt

@@ -234,6 +234,7 @@
  * Add the ability to disable bulk loading of SSTables (CASSANDRA-18781)
  * Clean up obsolete functions and simplify cql_version handling in cqlsh (CASSANDRA-18787)
 Merged from 5.0:
+ * Heap dump should not be generated on handled exceptions (CASSANDRA-20974)
  * Flush SAI segment builder when current SSTable writer is switched (CASSANDRA-20752)
  * Throw RTE instead of FSError when RTE is thrown from FileUtis.write in TOCComponent (CASSANDRA-20917)
  * Represent complex settings as JSON on system_views.settings table (CASSANDRA-20827)

src/java/org/apache/cassandra/utils/HeapUtils.java

@@ -24,7 +24,6 @@
 import java.lang.management.ManagementFactory;
 import java.nio.file.FileStore;
 import java.nio.file.Path;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 import javax.management.MBeanServer;
@@ -38,7 +37,6 @@
 import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.io.util.File;
 import org.apache.cassandra.io.util.PathUtils;
-import org.apache.cassandra.utils.NoSpamLogger.NoSpamLogStatement;
 
 import static org.apache.cassandra.config.CassandraRelevantEnv.JAVA_HOME;
 import static org.apache.cassandra.utils.Clock.Global.currentTimeMillis;
@@ -50,7 +48,6 @@
 public final class HeapUtils
 {
     private static final Logger logger = LoggerFactory.getLogger(HeapUtils.class);
-    private static final NoSpamLogStatement disabledStatement = NoSpamLogger.getStatement(logger, "Heap dump creation on uncaught exceptions is disabled.", 1L, TimeUnit.MINUTES);
 
     private static final Lock DUMP_LOCK = new ReentrantLock();
 
@@ -131,10 +128,6 @@ public static String maybeCreateHeapDump()
 
                     return fullPath;
                 }
-                else
-                {
-                    disabledStatement.debug();
-                }
             }
             catch (Throwable e)
             {

src/java/org/apache/cassandra/utils/JVMStabilityInspector.java

@@ -78,7 +78,7 @@ public static void uncaughtException(Thread thread, Throwable t)
             if (t2 != t && (t2 instanceof FSError || t2 instanceof CorruptSSTableException))
                 logger.error("Exception in thread {}", thread, t2);
         }
-        JVMStabilityInspector.inspectThrowable(t);
+        inspectThrowable(t, DiskErrorsHandlerService.get()::inspectDiskError, true);
     }
 
     /**
@@ -89,20 +89,20 @@ public static void uncaughtException(Thread thread, Throwable t)
      */
     public static void inspectThrowable(Throwable t) throws OutOfMemoryError
     {
-        inspectThrowable(t, DiskErrorsHandlerService.get()::inspectDiskError);
+        inspectThrowable(t, DiskErrorsHandlerService.get()::inspectDiskError, false);
     }
 
     public static void inspectCommitLogThrowable(Throwable t)
     {
-        inspectThrowable(t, ex -> DiskErrorsHandlerService.get().inspectCommitLogError(ex));
+        inspectThrowable(t, ex -> DiskErrorsHandlerService.get().inspectCommitLogError(ex), false);
     }
 
     public static void inspectJournalThrowable(Throwable t, String journalName, FailurePolicy failurePolicy)
     {
-        inspectThrowable(t, th -> inspectJournalError(th, journalName, failurePolicy));
+        inspectThrowable(t, th -> inspectJournalError(th, journalName, failurePolicy), false);
     }
 
-    public static void inspectThrowable(Throwable t, Consumer<Throwable> fn) throws OutOfMemoryError
+    public static void inspectThrowable(Throwable t, Consumer<Throwable> fn, boolean isUncaughtException) throws OutOfMemoryError
     {
         boolean isUnstable = false;
         if (t instanceof OutOfMemoryError)
@@ -136,13 +136,17 @@ else if (t instanceof UnrecoverableIllegalStateException)
         }
 
         // Anything other than an OOM, we should try and heap dump to capture what's going on if configured to do so
-        try
-        {
-            HeapUtils.maybeCreateHeapDump();
-        }
-        catch (Throwable sub)
+        if (isUncaughtException && DatabaseDescriptor.getDumpHeapOnUncaughtException())
         {
-            t.addSuppressed(sub);
+            try
+            {
+                // Avoid entering maybeCreateHeapDump unless the setting is enabled to avoid expensive lock
+                HeapUtils.maybeCreateHeapDump();
+            }
+            catch (Throwable sub)
+            {
+                t.addSuppressed(sub);
+            }
         }
 
         if (t instanceof InterruptedException)
@@ -177,7 +181,7 @@ else if (t instanceof UnrecoverableIllegalStateException)
         }
 
         if (t.getCause() != null)
-            inspectThrowable(t.getCause(), fn);
+            inspectThrowable(t.getCause(), fn, isUncaughtException);
     }
 
     private static final Set<String> FORCE_HEAP_OOM_IGNORE_SET = ImmutableSet.of("Java heap space", "GC Overhead limit exceeded");