EdColeman commented on code in PR #3508:
URL: https://github.com/apache/accumulo/pull/3508#discussion_r1240323603
##########
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);
}
}
+ LOG.info("Consumed memory freed");
+ }
+
+ @Override
+ @SuppressFBWarnings(value = "DM_GC", justification = "gc is okay for test")
+ public void init(SortedKeyValueIterator<Key,Value> source,
Map<String,String> options,
+ IteratorEnvironment env) throws IOException {
+ super.init(source, options, env);
+ LOG.info("init call - should free consumed memory");
Review Comment:
The init() method was removed in 55265e8199. The loop in the constructor is
testing for the low memory detector to clear (show available memory). So, if
the gc does not run, the the test will eventually time-out and fail, or the gc
will decide that it should run either naturally, or after repeated hint
requests it decides we really, really mean it.
Over-all, these tests are extremely slow to run. But it is also a difficult
situation to force and then test for recovery. Maybe we don't run these on a
regular basis? Maybe run a subset unless specifically requested? It does seem
that we should have some coverage of the low memory detector, but I do not know
what the balance is.
The low memory detector is broke right now. Adding these changes should fix
that. We can reevaluate the tests after this is committed, but these changes
do move the ball forward.
--
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]
