This is an automated email from the ASF dual-hosted git repository. dcapwell pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push: new b29e103 JVMStabilityInspector.forceHeapSpaceOomMaybe should handle all non-heap OOMs rather than only supporting direct only b29e103 is described below commit b29e1037e4da75dfd2a30ad10f8008b24941e37f Author: David Capwell <dcapw...@apache.org> AuthorDate: Mon Nov 8 16:03:06 2021 -0800 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 +- src/java/org/apache/cassandra/io/util/Memory.java | 2 +- .../cassandra/utils/JVMStabilityInspector.java | 33 ++++++++++------------ .../cassandra/utils/JVMStabilityInspectorTest.java | 10 +++++++ 5 files changed, 28 insertions(+), 20 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index a0a1a04..6ca209e 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 0c92167..4bc31dd 100644 --- a/checkstyle.xml +++ b/checkstyle.xml @@ -65,7 +65,7 @@ <property name="illegalClasses" value="java.io.File,java.io.FileInputStream,java.io.FileOutputStream,java.io.FileReader,java.io.FileWriter,java.io.RandomAccessFile,java.util.concurrent.Semaphore,java.util.concurrent.CountDownLatch,java.util.concurrent.Executors,java.util.concurrent.LinkedBlockingQueue,java.util.concurrent.SynchronousQueue,java.util.concurrent.ArrayBlockingQueue,com.google.common.util.concurrent.Futures,java.util.concurrent.CompletableFuture,io.netty.util.concurrent [...] </module> <module name="IllegalInstantiation"> - <property name="classes" value="java.io.File,java.lang.Thread,java.util.concurrent.FutureTask,java.util.concurrent.Semaphore,java.util.concurrent.CountDownLatch,java.util.concurrent.ScheduledThreadPoolExecutor,java.util.concurrent.ThreadPoolExecutor,java.util.concurrent.ForkJoinPool))"/> + <property name="classes" value="java.io.File,java.lang.Thread,java.util.concurrent.FutureTask,java.util.concurrent.Semaphore,java.util.concurrent.CountDownLatch,java.util.concurrent.ScheduledThreadPoolExecutor,java.util.concurrent.ThreadPoolExecutor,java.util.concurrent.ForkJoinPool,java.lang.OutOfMemoryError"/> </module> </module> diff --git a/src/java/org/apache/cassandra/io/util/Memory.java b/src/java/org/apache/cassandra/io/util/Memory.java index c55f047..7fd4225 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 6470f1f..f8acb22 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<String> 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<long[]> ignored = new ArrayList<>(); + while (true) { - logger.error("Force heap space OutOfMemoryError in the presence of", oom); - // Start to produce heap space OOM forcibly. - List<long[]> 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 a8dd22a..f86be38 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; @@ -126,6 +127,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() { KillerForTests killerForTests = new KillerForTests(); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org