ctubbsii commented on code in PR #3508:
URL: https://github.com/apache/accumulo/pull/3508#discussion_r1240330837
##########
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:
> So, if the gc does not run, the the test will eventually time-out and fail
Resulting in a flaky test. That's my concern.
> Maybe we don't run these on a regular basis?
Running with the full ITs is fine. They shouldn't be running with sunny,
though.
> We can reevaluate the tests after this is committed, but these changes do
move the ball forward.
Absolutely agree :+1: If the test is flaky, we'll deal with it then.
--
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]
