SOLR-4974, tighten up shutdown logic. Inspired by SOLR-4960

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1497999 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -221,6 +221,9 @@ Bug Fixes
 
 * SOLR-4926: Fixed rare replication bug that normally only manifested when 
   using compound file format. (yonik, Mark Miller)
+  
+* SOLR-4974: Outgrowth of SOLR-4960 that includes transient cores and pending cores
+  (Erick Erickson)
 
 Optimizations
 ----------------------

solr/core/src/java/org/apache/solr/core/SolrCores.java

@@ -101,25 +101,25 @@ class SolrCores {
   // We are shutting down. You can't hold the lock on the various lists of cores while they shut down, so we need to
   // make a temporary copy of the names and shut them down outside the lock.
   protected void close() {
-    Collection<SolrCore> coreList;
+    Collection<SolrCore> coreList = new ArrayList<SolrCore>();
-    List<String> transientNames;
-    List<SolrCore> pendingToClose;
 
     // It might be possible for one of the cores to move from one list to another while we're closing them. So
     // loop through the lists until they're all empty. In particular, the core could have moved from the transient
     // list to the pendingCloses list.
 
-    while (true) {
+    do {
+      coreList.clear();
       synchronized (modifyLock) {
         // make a copy of the cores then clear the map so the core isn't handed out to a request again
-        coreList = new ArrayList<SolrCore>(cores.values());
+        coreList.addAll(cores.values());
         cores.clear();
 
-        transientNames = new ArrayList<String>(transientCores.keySet());
+        coreList.addAll(transientCores.values());
-        pendingToClose = new ArrayList<SolrCore>(pendingCloses);
+        transientCores.clear();
-      }
 
-      if (coreList.size() == 0 && transientNames.size() == 0 && pendingToClose.size() == 0) break;
+        coreList.addAll(pendingCloses);
+        pendingCloses.clear();
+      }
 
       for (SolrCore core : coreList) {
         try {
@@ -128,37 +128,7 @@ class SolrCores {
           SolrException.log(CoreContainer.log, "Error shutting down core", t);
         }
       }
-
+    } while (coreList.size() > 0);
-      for (String coreName : transientNames) {
-        SolrCore core = transientCores.get(coreName);
-        if (core == null) {
-          CoreContainer.log.info("Core " + coreName + " moved from transient core container list before closing.");
-        } else {
-          try {
-            core.close();
-          } catch (Throwable t) {
-            SolrException.log(CoreContainer.log, "Error shutting down core", t);
-          } finally {
-            synchronized (modifyLock) {
-              transientCores.remove(coreName);
-            }
-          }
-        }
-      }
-
-      // We might have some cores that we were _thinking_ about shutting down, so take care of those too.
-      for (SolrCore core : pendingToClose) {
-        try {
-          core.close();
-        } catch (Throwable t) {
-          SolrException.log(CoreContainer.log, "Error shutting down core", t);
-        } finally {
-          synchronized (modifyLock) {
-            pendingCloses.remove(core);
-          }
-        }
-      }
-    }
   }
 
   //WARNING! This should be the _only_ place you put anything into the list of transient cores!
@@ -304,7 +274,7 @@ class SolrCores {
   }
 
   /* If you don't increment the reference count, someone could close the core before you use it. */
-  protected SolrCore getCoreFromAnyList(String name, boolean incRefCount) {
+  protected SolrCore  getCoreFromAnyList(String name, boolean incRefCount) {
     synchronized (modifyLock) {
       SolrCore core = cores.get(name);
 

solr/core/src/test-files/solr/collection1/conf/solrconfig-minimal.xml

@@ -28,7 +28,7 @@
   <directoryFactory name="DirectoryFactory"
                     class="${solr.directoryFactory:solr.NRTCachingDirectoryFactory}"/>
 
-  <xi:include href="solrconfig.snippet.randomindexconfig.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
+  <xi:include href="./solrconfig.snippet.randomindexconfig.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
 
   <jmx/>
   <updateHandler class="solr.DirectUpdateHandler2">

solr/core/src/test/org/apache/solr/core/OpenCloseCoreStressTest.java

@@ -207,9 +207,10 @@ public class OpenCloseCoreStressTest extends SolrTestCaseJ4 {
         }
       } while (secondsRemaining > 0);
 
-      assertTrue("We didn't index any documents, somethings really messsed up", cumulativeDocs > 0);
+      assertTrue("We didn't index any documents, somethings really messed up", cumulativeDocs > 0);
     } catch (Exception e) {
       e.printStackTrace();
+      fail("Caught unexpected exception");
     }
   }
 
@@ -241,6 +242,8 @@ public class OpenCloseCoreStressTest extends SolrTestCaseJ4 {
     FileUtils.copyFile(new File(testConf, "schema-tiny.xml"), new File(conf, "schema-tiny.xml"));
 
     FileUtils.copyFile(new File(testConf, "solrconfig-minimal.xml"), new File(conf, "solrconfig-minimal.xml"));
+    FileUtils.copyFile(new File(testConf, "solrconfig.snippet.randomindexconfig.xml"),
+        new File(conf, "solrconfig.snippet.randomindexconfig.xml"));
 
     if (!oldStyle) {
       FileUtils.copyFile(new File(testSrcRoot, "conf/core.properties"), new File(coreDir, "core.properties"));
@@ -479,7 +482,7 @@ class Queries {
     try {
       QueryResponse response = server.query(params);
       numFound = response.getResults().getNumFound();
-    } catch (SolrServerException e) {
+    } catch (Exception e) {
       e.printStackTrace();
     }
     return numFound;