ctubbsii commented on code in PR #3508:
URL: https://github.com/apache/accumulo/pull/3508#discussion_r1239325100
##########
test/src/main/java/org/apache/accumulo/test/functional/MemoryFreeingIterator.java:
##########
@@ -28,23 +28,39 @@
import org.apache.accumulo.core.iterators.IteratorEnvironment;
import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
import org.apache.accumulo.core.iterators.WrappingIterator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
public class MemoryFreeingIterator extends WrappingIterator {
- @Override
- public void init(SortedKeyValueIterator<Key,Value> source,
Map<String,String> options,
- IteratorEnvironment env) throws IOException {
- super.init(source, options, env);
+ private static final Logger LOG =
LoggerFactory.getLogger(MemoryFreeingIterator.class);
+
+ @SuppressFBWarnings(value = "DM_GC", justification = "gc is okay for test")
+ public MemoryFreeingIterator() {
+ super();
+ LOG.info("Freeing consumed memory");
MemoryConsumingIterator.freeBuffers();
while (this.isRunningLowOnMemory()) {
+ System.gc();
// wait for LowMemoryDetector to recognize the memory is free.
try {
Thread.sleep(SECONDS.toMillis(1));
} catch (InterruptedException ex) {
Thread.currentThread().interrupt();
- throw new IOException("wait for low memory detector interrupted", ex);
+ throw new RuntimeException("wait for low memory detector interrupted",
ex);
}
Review Comment:
No. I'm still seeing the IOException changed to RuntimeException in this PR.
I'm suggesting just get rid of the catch block entirely, and put `throws
InterruptedException` on the method, because that's good enough for the test.
The only thing this catch block is doing is adding noise to the test code to
add an unnecessary conversion of an interrupt to a different exception type and
adding a message to that new exception that doesn't provide more useful details
than the stack trace would already have. The interrupt isn't even likely to
happen anyway, unless the user kills the test on purpose, in which case, they
probably don't care which exception type gets thrown.
--
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]
