From b29e1037e4da75dfd2a30ad10f8008b24941e37f Mon Sep 17 00:00:00 2001 From: David Capwell Date: Mon, 8 Nov 2021 16:03:06 -0800 Subject: [PATCH] JVMStabilityInspector.forceHeapSpaceOomMaybe should handle all non-heap OOMs rather than only supporting direct only patch by David Capwell; reviewed by Caleb Rackliffe, Yifan Cai for CASSANDRA-17128 --- CHANGES.txt | 1 + checkstyle.xml | 2 +- .../org/apache/cassandra/io/util/Memory.java | 2 +- .../utils/JVMStabilityInspector.java | 33 +++++++++---------- .../utils/JVMStabilityInspectorTest.java | 10 ++++++ 5 files changed, 28 insertions(+), 20 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index a0a1a0481d..6ca209e000 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1 + * JVMStabilityInspector.forceHeapSpaceOomMaybe should handle all non-heap OOMs rather than only supporting direct only (CASSANDRA-17128) * Forbid other Future implementations with checkstyle (CASSANDRA-17055) * commit log was switched from non-daemon to daemon threads, which causes the JVM to exit in some case as no non-daemon threads are active (CASSANDRA-17085) * Add a Denylist to block reads and writes on specific partition keys (CASSANDRA-12106) diff --git a/checkstyle.xml b/checkstyle.xml index 0c921678ee..4bc31dd348 100644 --- a/checkstyle.xml +++ b/checkstyle.xml @@ -65,7 +65,7 @@ - + diff --git a/src/java/org/apache/cassandra/io/util/Memory.java b/src/java/org/apache/cassandra/io/util/Memory.java index c55f047949..7fd4225ec0 100644 --- a/src/java/org/apache/cassandra/io/util/Memory.java +++ b/src/java/org/apache/cassandra/io/util/Memory.java @@ -68,7 +68,7 @@ public class Memory implements AutoCloseable, ReadableMemory // we permit a 0 peer iff size is zero, since such an allocation makes no sense, and an allocator would be // justified in returning a null pointer (and permitted to do so: http://www.cplusplus.com/reference/cstdlib/malloc) if (peer == 0) - throw new OutOfMemoryError(); + throw new OutOfMemoryError(); // checkstyle: permit this instantiation } // create a memory object that references the exacy same memory location as the one provided. diff --git a/src/java/org/apache/cassandra/utils/JVMStabilityInspector.java b/src/java/org/apache/cassandra/utils/JVMStabilityInspector.java index 6470f1ff3d..f8acb226ce 100644 --- a/src/java/org/apache/cassandra/utils/JVMStabilityInspector.java +++ b/src/java/org/apache/cassandra/utils/JVMStabilityInspector.java @@ -21,13 +21,14 @@ import java.io.FileNotFoundException; import java.net.SocketException; import java.nio.file.FileSystemException; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableSet; import org.apache.cassandra.exceptions.UnrecoverableIllegalStateException; import org.apache.cassandra.metrics.StorageMetrics; @@ -153,31 +154,27 @@ public final class JVMStabilityInspector inspectThrowable(t.getCause(), fn); } + private static final Set FORCE_HEAP_OOM_IGNORE_SET = ImmutableSet.of("Java heap space", "GC Overhead limit exceeded"); + /** - * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM. + * Intentionally produce a heap space OOM upon seeing a non heap memory OOM. * Direct buffer OOM cannot trigger JVM OOM error related options, * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc. - * See CASSANDRA-15214 for more details + * See CASSANDRA-15214 and CASSANDRA-17128 for more details */ @Exclude // Exclude from just in time compilation. private static void forceHeapSpaceOomMaybe(OutOfMemoryError oom) { - // See the oom thrown from java.nio.Bits.reserveMemory. - // In jdk 13 and up, the message is "Cannot reserve XX bytes of direct buffer memory (...)" - // In jdk 11 and below, the message is "Direct buffer memory" - if ((oom.getMessage() != null && oom.getMessage().toLowerCase().contains("direct buffer memory")) || - Arrays.stream(oom.getStackTrace()).anyMatch(x -> x.getClassName().equals("java.nio.Bits") - && x.getMethodName().equals("reserveMemory"))) + if (FORCE_HEAP_OOM_IGNORE_SET.contains(oom.getMessage())) + return; + logger.error("Force heap space OutOfMemoryError in the presence of", oom); + // Start to produce heap space OOM forcibly. + List ignored = new ArrayList<>(); + while (true) { - logger.error("Force heap space OutOfMemoryError in the presence of", oom); - // Start to produce heap space OOM forcibly. - List ignored = new ArrayList<>(); - while (true) - { - // java.util.AbstractCollection.MAX_ARRAY_SIZE is defined as Integer.MAX_VALUE - 8 - // so Integer.MAX_VALUE / 2 should be a large enough and safe size to request. - ignored.add(new long[Integer.MAX_VALUE / 2]); - } + // java.util.AbstractCollection.MAX_ARRAY_SIZE is defined as Integer.MAX_VALUE - 8 + // so Integer.MAX_VALUE / 2 should be a large enough and safe size to request. + ignored.add(new long[Integer.MAX_VALUE / 2]); } } diff --git a/test/unit/org/apache/cassandra/utils/JVMStabilityInspectorTest.java b/test/unit/org/apache/cassandra/utils/JVMStabilityInspectorTest.java index a8dd22a3fb..f86be383ae 100644 --- a/test/unit/org/apache/cassandra/utils/JVMStabilityInspectorTest.java +++ b/test/unit/org/apache/cassandra/utils/JVMStabilityInspectorTest.java @@ -29,6 +29,7 @@ import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.io.FSReadError; import org.apache.cassandra.io.FSWriteError; import org.apache.cassandra.io.sstable.CorruptSSTableException; +import org.assertj.core.api.Assertions; import static java.util.Arrays.asList; import static org.junit.Assert.assertEquals; @@ -125,6 +126,15 @@ public class JVMStabilityInspectorTest } } + @Test + public void testForceHeapSpaceOomExclude() + { + OutOfMemoryError error = new OutOfMemoryError("Java heap space"); + Assertions.assertThatThrownBy(() -> JVMStabilityInspector.inspectThrowable(error)) + .isInstanceOf(OutOfMemoryError.class) + .isEqualTo(error); + } + @Test public void fileHandleTest() {