From e35881a63aa9de72cf5c539396266e0d0e676956 Mon Sep 17 00:00:00 2001 From: Mark Miller Date: Wed, 8 Mar 2017 11:30:08 -0500 Subject: [PATCH] SOLR-10248: Merge SolrTestCaseJ4's SolrIndexSearcher tracking into the ObjectReleaseTracker. --- .../apache/solr/search/SolrIndexSearcher.java | 3 + .../java/org/apache/solr/SolrTestCaseJ4.java | 71 +++++-------------- 2 files changed, 19 insertions(+), 55 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java index 521324a0ed6..a7ee433f4d1 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java +++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java @@ -105,6 +105,7 @@ import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.ObjectReleaseTracker; import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.core.DirectoryFactory; import org.apache.solr.core.DirectoryFactory.DirContext; @@ -391,6 +392,7 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI readerStats = snapStatistics(reader); // do this at the end since an exception in the constructor means we won't close numOpens.incrementAndGet(); + assert ObjectReleaseTracker.track(this); } /* @@ -539,6 +541,7 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI // do this at the end so it only gets done if there are no exceptions numCloses.incrementAndGet(); + assert ObjectReleaseTracker.release(this); } /** Direct access to the IndexSchema for use with this searcher */ diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java index a8c93d68f06..825e7c762b0 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java +++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java @@ -297,17 +297,10 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { if (suiteFailureMarker.wasSuccessful()) { // if the tests passed, make sure everything was closed / released if (!RandomizedContext.current().getTargetClass().isAnnotationPresent(SuppressObjectReleaseTracker.class)) { - endTrackingSearchers(120, false); - String orr = clearObjectTrackerAndCheckEmpty(120); + String orr = clearObjectTrackerAndCheckEmpty(20, false); assertNull(orr, orr); } else { - endTrackingSearchers(15, false); - String orr = ObjectReleaseTracker.checkEmpty(); - if (orr != null) { - log.warn( - "Some resources were not closed, shutdown, or released. This has been ignored due to the SuppressObjectReleaseTracker annotation, trying to close them now."); - ObjectReleaseTracker.tryClose(); - } + clearObjectTrackerAndCheckEmpty(20, true); } } resetFactory(); @@ -341,6 +334,13 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { * @return null if ok else error message */ public static String clearObjectTrackerAndCheckEmpty(int waitSeconds) { + return clearObjectTrackerAndCheckEmpty(waitSeconds, false); + } + + /** + * @return null if ok else error message + */ + public static String clearObjectTrackerAndCheckEmpty(int waitSeconds, boolean tryClose) { int retries = 0; String result; do { @@ -367,6 +367,13 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { log.info("------------------------------------------------------- Done waiting for tracked resources to be released"); + + if (tryClose && result != null && RandomizedContext.current().getTargetClass().isAnnotationPresent(SuppressObjectReleaseTracker.class)) { + log.warn( + "Some resources were not closed, shutdown, or released. This has been ignored due to the SuppressObjectReleaseTracker annotation, trying to close them now."); + ObjectReleaseTracker.tryClose(); + } + ObjectReleaseTracker.clear(); return result; @@ -580,52 +587,6 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase { numOpens = numCloses = 0; } } - - public static void endTrackingSearchers(int waitSeconds, boolean failTest) { - long endNumOpens = SolrIndexSearcher.numOpens.get(); - long endNumCloses = SolrIndexSearcher.numCloses.get(); - - // wait a bit in case any ending threads have anything to release - int retries = 0; - while (endNumOpens - numOpens != endNumCloses - numCloses) { - if (retries++ > waitSeconds) { - break; - } - if (retries % 10 == 0) { - log.info("Waiting for all SolrIndexSearchers to be released at end of test"); - if (retries > 10) { - TraceFormatting tf = new TraceFormatting(); - Map stacksMap = Thread.getAllStackTraces(); - Set> entries = stacksMap.entrySet(); - for (Entry entry : entries) { - String stack = tf.formatStackTrace(entry.getValue()); - System.err.println(entry.getKey().getName() + ":\n" + stack); - } - } - } - try { - Thread.sleep(1000); - } catch (InterruptedException e) {} - endNumOpens = SolrIndexSearcher.numOpens.get(); - endNumCloses = SolrIndexSearcher.numCloses.get(); - } - - log.info("------------------------------------------------------- Done waiting for all SolrIndexSearchers to be released"); - - SolrIndexSearcher.numOpens.getAndSet(0); - SolrIndexSearcher.numCloses.getAndSet(0); - - if (endNumOpens-numOpens != endNumCloses-numCloses) { - String msg = "ERROR: SolrIndexSearcher opens=" + (endNumOpens-numOpens) + " closes=" + (endNumCloses-numCloses); - log.error(msg); - // if it's TestReplicationHandler, ignore it. the test is broken and gets no love - if ("TestReplicationHandler".equals(RandomizedContext.current().getTargetClass().getSimpleName())) { - log.warn("TestReplicationHandler wants to fail!: " + msg); - } else { - if (failTest) fail(msg); - } - } - } /** Causes an exception matching the regex pattern to not be logged. */ public static void ignoreException(String pattern) {