You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/09/29 22:23:22 UTC
[GitHub] [cassandra] yifan-c opened a new pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory Error
yifan-c opened a new pull request #761:
URL: https://github.com/apache/cassandra/pull/761
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jrwest commented on a change in pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory error
Posted by GitBox <gi...@apache.org>.
jrwest commented on a change in pull request #761:
URL: https://github.com/apache/cassandra/pull/761#discussion_r521493650
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +123,31 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ * See CASSANDRA-15214 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.
+ if (!"Direct buffer memory".equals(oom.getMessage()))
+ {
+ return;
+ }
+ logger.error("Force heap space OutOfMemoryError in the presence of", oom);
+ List<long[]> ignored = new ArrayList<>();
Review comment:
It took me a sec to realize the goal here is to literally OOM the process. It might be nice to add a comment before the loop to that effect.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #761:
URL: https://github.com/apache/cassandra/pull/761#discussion_r517678215
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +120,28 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ */
+ private static void forceHeapSpaceOomMaybe(OutOfMemoryError oom)
+ {
+ // See the oom thrown from java.nio.Bits.reserveMemory.
+ if (oom.getMessage() != null && !oom.getMessage().equals("Direct buffer memory"))
Review comment:
can simplify to the below as string equality checks null, so don't need a null check first.
```
if (!"Direct buffer memory".equals(oom.getMessage()))
```
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +120,28 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ */
+ private static void forceHeapSpaceOomMaybe(OutOfMemoryError oom)
+ {
+ // See the oom thrown from java.nio.Bits.reserveMemory.
+ if (oom.getMessage() != null && !oom.getMessage().equals("Direct buffer memory"))
+ {
+ return;
+ }
+ logger.warn("Force heap space OutOfMemoryError in the presence of", oom);
+ 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.
+ long[] ignored = new long[Integer.MAX_VALUE / 2];
Review comment:
it may be good to collect the references in a list to make sure GC isn't able to free right away.
```
List<long[]> ignored = new ArrayList<>();
while (true)
{
ignored.add(new long[Integer.MAX_VALUE / 2]);
}
```
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +120,28 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ */
+ private static void forceHeapSpaceOomMaybe(OutOfMemoryError oom)
+ {
+ // See the oom thrown from java.nio.Bits.reserveMemory.
+ if (oom.getMessage() != null && !oom.getMessage().equals("Direct buffer memory"))
+ {
+ return;
+ }
+ logger.warn("Force heap space OutOfMemoryError in the presence of", oom);
Review comment:
thinking should be `error`
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +120,28 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ */
Review comment:
would be good to link to `CASSANDRA-15214`, something like
```
* see CASSANDRA-15214 for more details
```
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +120,28 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ */
+ private static void forceHeapSpaceOomMaybe(OutOfMemoryError oom)
+ {
+ // See the oom thrown from java.nio.Bits.reserveMemory.
+ if (oom.getMessage() != null && !oom.getMessage().equals("Direct buffer memory"))
+ {
+ return;
+ }
+ logger.warn("Force heap space OutOfMemoryError in the presence of", oom);
+ 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.
+ long[] ignored = new long[Integer.MAX_VALUE / 2];
Review comment:
Also, in case the while true loop never breaks (such as JVM gets smart enough to know this is not needed and avoids the allocation), we should probably start logging an error saying that this trick isn't doing what we hoped.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jrwest commented on a change in pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory error
Posted by GitBox <gi...@apache.org>.
jrwest commented on a change in pull request #761:
URL: https://github.com/apache/cassandra/pull/761#discussion_r521492490
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +123,31 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ * See CASSANDRA-15214 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.
+ if (!"Direct buffer memory".equals(oom.getMessage()))
Review comment:
Depending on a string from JVM internal libraries seems like it has the potential to break easily in the future. Unless the Error Strings are something the JVM API is committed to keeping (which I don't have any recollection of being the case).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] yifan-c commented on a change in pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory error
Posted by GitBox <gi...@apache.org>.
yifan-c commented on a change in pull request #761:
URL: https://github.com/apache/cassandra/pull/761#discussion_r521564779
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +123,31 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ * See CASSANDRA-15214 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.
+ if (!"Direct buffer memory".equals(oom.getMessage()))
Review comment:
A downside is that checking trace is more expensive.
I can update the condition to short circuit when the message contains `"direct buffer memory"`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] yifan-c commented on a change in pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory error
Posted by GitBox <gi...@apache.org>.
yifan-c commented on a change in pull request #761:
URL: https://github.com/apache/cassandra/pull/761#discussion_r521552554
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +123,31 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ * See CASSANDRA-15214 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.
+ if (!"Direct buffer memory".equals(oom.getMessage()))
Review comment:
Agree. It is a hacky approach.
The message remains the same for jdk 11 and below.
But the error message has changed since jdk 13. [[1]](https://github.com/openjdk/jdk13u/blob/a0651dd95b40c41803c938781f810aa40c43737b/src/java.base/share/classes/java/nio/Bits.java#L175), [[2]](https://github.com/openjdk/jdk15u/blob/ac465695d5924dc2a634d660c60f2d54d3d2743d/src/java.base/share/classes/java/nio/Bits.java#L175)
I think we can improve it by checking the stack trace that the OOM is thrown from `java.nio.Bits. reserveMemory`. It is more future proofing (valid through jdk15 at least). But still a hack though.
WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] smiklosovic closed pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory error
Posted by GitBox <gi...@apache.org>.
smiklosovic closed pull request #761:
URL: https://github.com/apache/cassandra/pull/761
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory error
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #761:
URL: https://github.com/apache/cassandra/pull/761#discussion_r518973639
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +120,28 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ */
+ private static void forceHeapSpaceOomMaybe(OutOfMemoryError oom)
+ {
+ // See the oom thrown from java.nio.Bits.reserveMemory.
+ if (oom.getMessage() != null && !oom.getMessage().equals("Direct buffer memory"))
+ {
+ return;
+ }
+ logger.warn("Force heap space OutOfMemoryError in the presence of", oom);
+ 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.
+ long[] ignored = new long[Integer.MAX_VALUE / 2];
Review comment:
spoke in slack, adding `@Exclude` to the method should remove the concern of "smartness" trying to avoid the allocation
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] jrwest commented on a change in pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory error
Posted by GitBox <gi...@apache.org>.
jrwest commented on a change in pull request #761:
URL: https://github.com/apache/cassandra/pull/761#discussion_r521560937
##########
File path: src/java/org/apache/cassandra/utils/JVMStabilityInspector.java
##########
@@ -125,7 +123,31 @@ public static void inspectThrowable(Throwable t, boolean propagateOutOfMemory, C
killer.killCurrentJVM(t);
if (t.getCause() != null)
- inspectThrowable(t.getCause(), propagateOutOfMemory, fn);
+ inspectThrowable(t.getCause(), fn);
+ }
+
+ /**
+ * Intentionally produce a heap space OOM upon seeing a Direct buffer memory OOM.
+ * Direct buffer OOM cannot trigger JVM OOM error related options,
+ * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
+ * See CASSANDRA-15214 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.
+ if (!"Direct buffer memory".equals(oom.getMessage()))
Review comment:
That’s more what I had in mind, yes. Not perfect but more future proof.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] yifan-c commented on pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory error
Posted by GitBox <gi...@apache.org>.
yifan-c commented on pull request #761:
URL: https://github.com/apache/cassandra/pull/761#issuecomment-723316645
Thanks @dcapwell.
For the exception handler, how about having the inspector at the end of the method. so it makes the best effort to send back the error message then handles the throwable.
Since `inspectThrowable` rethrows and netty swallows any unhandled exceptions. I will just catch any re-thrown throwable and log, as [here](https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/net/InboundConnectionInitiator.java#L347-L350) does.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] yifan-c edited a comment on pull request #761: CASSANDRA-15214: Force throw heap space OutOfMemory error
Posted by GitBox <gi...@apache.org>.
yifan-c edited a comment on pull request #761:
URL: https://github.com/apache/cassandra/pull/761#issuecomment-723316645
Thanks @dcapwell.
For the exception handler, how about having the inspector at the end of the method? So it makes the best effort to send back the error message then handles the throwable.
Since `inspectThrowable` rethrows and netty swallows any unhandled exceptions. I will just catch any re-thrown throwable and log, as [here](https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/net/InboundConnectionInitiator.java#L347-L350) does.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org