You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "Maxwell-Guo (via GitHub)" <gi...@apache.org> on 2023/04/18 02:06:52 UTC
[GitHub] [cassandra] Maxwell-Guo commented on a diff in pull request #2276: CASSANDRA-18336
Maxwell-Guo commented on code in PR #2276:
URL: https://github.com/apache/cassandra/pull/2276#discussion_r1169415630
##########
test/unit/org/apache/cassandra/service/DiskFailurePolicyTest.java:
##########
@@ -79,22 +88,24 @@ public DiskFailurePolicyTest(DiskFailurePolicy testPolicy, boolean isStartUpInPr
public static Collection<Object[]> generateData()
{
return Arrays.asList(new Object[][]{
- { Config.DiskFailurePolicy.die, true, new FSReadError(new IOException(), "blah"), false, true, true},
- { DiskFailurePolicy.ignore, true, new FSReadError(new IOException(), "blah"), true, false, false},
- { DiskFailurePolicy.stop, true, new FSReadError(new IOException(), "blah"), false, true, true},
- { DiskFailurePolicy.stop_paranoid, true, new FSReadError(new IOException(), "blah"), false, true, true},
- { Config.DiskFailurePolicy.die, true, new CorruptSSTableException(new IOException(), "blah"), false, true, true},
- { DiskFailurePolicy.ignore, true, new CorruptSSTableException(new IOException(), "blah"), true, false, false},
- { DiskFailurePolicy.stop, true, new CorruptSSTableException(new IOException(), "blah"), false, true, true},
- { DiskFailurePolicy.stop_paranoid, true, new CorruptSSTableException(new IOException(), "blah"), false, true, true},
- { Config.DiskFailurePolicy.die, false, new FSReadError(new IOException(), "blah"), false, true, false},
- { DiskFailurePolicy.ignore, false, new FSReadError(new IOException(), "blah"), true, false, false},
- { DiskFailurePolicy.stop, false, new FSReadError(new IOException(), "blah"), false, false, false},
- { DiskFailurePolicy.stop_paranoid, false, new FSReadError(new IOException(), "blah"), false, false, false},
- { Config.DiskFailurePolicy.die, false, new CorruptSSTableException(new IOException(), "blah"), false, true, false},
- { DiskFailurePolicy.ignore, false, new CorruptSSTableException(new IOException(), "blah"), true, false, false},
- { DiskFailurePolicy.stop, false, new CorruptSSTableException(new IOException(), "blah"), true, false, false},
- { DiskFailurePolicy.stop_paranoid, false, new CorruptSSTableException(new IOException(), "blah"), false, false, false}
+ { die, true, new FSReadError(new IOException(), "blah"), false, true, true},
+ { ignore, true, new FSReadError(new IOException(), "blah"), true, false, false},
+ { stop, true, new FSReadError(new IOException(), "blah"), false, true, true},
+ { stop_paranoid, true, new FSReadError(new IOException(), "blah"), false, true, true},
+ { die, true, new CorruptSSTableException(new IOException(), "blah"), false, true, true},
+ { ignore, true, new CorruptSSTableException(new IOException(), "blah"), true, false, false},
+ { stop, true, new CorruptSSTableException(new IOException(), "blah"), false, true, true},
+ { stop_paranoid, true, new CorruptSSTableException(new IOException(), "blah"), false, true, true},
+ { die, false, new FSReadError(new IOException(), "blah"), false, true, false},
+ { ignore, false, new FSReadError(new IOException(), "blah"), true, false, false},
+ { stop, false, new FSReadError(new IOException(), "blah"), false, false, false},
+ { stop_paranoid, false, new FSReadError(new IOException(), "blah"), false, false, false},
+ { die, false, new CorruptSSTableException(new IOException(), "blah"), false, true, false},
+ { ignore, false, new CorruptSSTableException(new IOException(), "blah"), true, false, false},
+ { stop, false, new CorruptSSTableException(new IOException(), "blah"), true, false, false},
+ { stop_paranoid, false, new CorruptSSTableException(new IOException(), "blah"), false, false, false},
+ { best_effort, false, new FSReadError(new IOException(new OutOfMemoryError("Java heap space test")), "best_effort_oom"), true, false, false},
Review Comment:
as "Java heap space test" removed in FORCE_HEAP_OOM_IGNORE_SET , so I think some other like "Java heap space" can be used here. I don't know if this is the right suggestion.
##########
src/java/org/apache/cassandra/service/DefaultFSErrorHandler.java:
##########
@@ -96,6 +96,20 @@ public void handleFSError(FSError e)
}
}
+ private boolean isCausedByOutOfMemoryException(Throwable error)
+ {
+ for (Throwable t = error; t != null; t = t.getCause())
+ {
+ if (t instanceof OutOfMemoryError)
+ return true;
+ for (Throwable s : t.getSuppressed())
+ if (s instanceof OutOfMemoryError)
+ return true;
Review Comment:
code format : the "return true;" should have a "tab" before ?
`if (s instanceof OutOfMemoryError)
return true;`
##########
src/java/org/apache/cassandra/utils/JVMStabilityInspector.java:
##########
@@ -168,7 +168,7 @@ else if (t instanceof UnrecoverableIllegalStateException)
inspectThrowable(t.getCause(), fn);
}
- private static final Set<String> FORCE_HEAP_OOM_IGNORE_SET = ImmutableSet.of("Java heap space", "GC Overhead limit exceeded");
+ private static final Set<String> FORCE_HEAP_OOM_IGNORE_SET = ImmutableSet.of("Java heap space", "GC Overhead limit exceeded", "Java heap space test");
Review Comment:
I think "Java heap space test" may be not needed here, as FORCE_HEAP_OOM_IGNORE_SET is not used in the ut.
##########
src/java/org/apache/cassandra/service/DefaultFSErrorHandler.java:
##########
@@ -81,7 +81,7 @@ public void handleFSError(FSError e)
// for both read and write errors mark the path as unwritable.
DisallowedDirectories.maybeMarkUnwritable(new File(e.path));
- if (e instanceof FSReadError)
+ if (e instanceof FSReadError && !isCausedByOutOfMemoryException(e))
Review Comment:
what about change the function to "isNotCausedByOutOfMemeoryException", so no "!" is needed before the function ,also the function logic should be rewrite ~~~~
--
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