liuisaac commented on code in PR #4916:
URL: https://github.com/apache/cassandra/pull/4916#discussion_r3533458999


##########
src/java/org/apache/cassandra/utils/JVMStabilityInspector.java:
##########
@@ -200,7 +200,7 @@ private static void forceHeapSpaceOomMaybe(OutOfMemoryError 
oom)
     {
         if (FORCE_HEAP_OOM_IGNORE_SET.contains(oom.getMessage()))
             return;
-        logger.error("Force heap space OutOfMemoryError in the presence of", 
oom);
+        logger.error("Non-heap OutOfMemoryError detected, forcing a heap 
OutOfMemoryError to trigger standard JVM OOM handling:", oom);

Review Comment:
   Should not be applicable here, non heap memory OOM is specified in the 
method's invariant:
   
   ```java
       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 non heap memory 
OOM.
        * Direct buffer OOM cannot trigger JVM OOM error related options,
        * e.g. OnOutOfMemoryError, HeapDumpOnOutOfMemoryError, etc.
        * See CASSANDRA-15214 and CASSANDRA-17128 for more details
        */
       @Exclude // Exclude from just in time compilation.
       private static void forceHeapSpaceOomMaybe(OutOfMemoryError oom)
       {
           if (FORCE_HEAP_OOM_IGNORE_SET.contains(oom.getMessage()))
               return;
   ```
   
   It is expected that this method only is invoked on non-heap OOMs, it should 
not be the job of the error message to account for misuse of the method.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to