SOLR-5366: transient cores automatically unloaded when creating cores via admin API would be closed too many times

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

solr/CHANGES.txt

@@ -153,9 +153,13 @@ Bug Fixes
 * SOLR-5418: Background merge after field removed from solr.xml causes error. 
   (Reported on user's list, Robert M's patch via Erick Erickson)
   
- * SOLR-5318: Creating a core via the admin API doesn't respect transient property
+* SOLR-5318: Creating a core via the admin API doesn't respect transient property
    (Olivier Soyez via Erick Erickson)
 
+* SOLR-5388: Creating a new core via the HTTP API that results in a transient being
+  unloaded results in a " Too many close [count:-1]" error.
+  (Olivier Soyez via Erick Erickson)
+
 Optimizations
 ----------------------  
 

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

@@ -923,6 +923,10 @@ public class CoreContainer {
     return solrCores.isLoaded(name);
   }
 
+  public boolean isLoadedNotPendingClose(String name) {
+    return solrCores.isLoadedNotPendingClose(name);
+  }
+
   /**
    * Gets a solr core descriptor for a core that is not loaded. Note that if the caller calls this on a
    * loaded core, the unloaded descriptor will be returned.

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

@@ -288,6 +288,29 @@ class SolrCores {
     }
   }
 
+  // See SOLR-5366 for why the UNLOAD command needs to know whether a core is actually loaded or not, it might have
+  // to close the core. However, there's a race condition. If the core happens to be in the pending "to close" queue,
+  // we should NOT close it in unload core.
+  protected boolean isLoadedNotPendingClose(String name) {
+    // Just all be synchronized
+    synchronized (modifyLock) {
+      if (cores.containsKey(name)) {
+        return true;
+      }
+      if (transientCores.containsKey(name)) {
+        // Check pending
+        for (SolrCore core : pendingCloses) {
+          if (core.getName().equals(name)) {
+            return false;
+          }
+        }
+
+        return true;
+      }
+    }
+    return false;
+  }
+
   protected boolean isLoaded(String name) {
     synchronized (modifyLock) {
       if (cores.containsKey(name)) {

solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java

@@ -582,6 +582,10 @@ public class CoreAdminHandler extends RequestHandlerBase {
       SolrQueryResponse rsp) throws SolrException {
     SolrParams params = req.getParams();
     String cname = params.get(CoreAdminParams.CORE);
+    Boolean closeCore = true;
+    if (!coreContainer.isLoadedNotPendingClose(cname)) {
+      closeCore = false;
+    }
     SolrCore core = coreContainer.remove(cname);
     try {
       if (core == null) {
@@ -653,7 +657,9 @@ public class CoreAdminHandler extends RequestHandlerBase {
         if (coreContainer.getZkController() != null) {
           core.getSolrCoreState().cancelRecovery();
         }
-        core.close();
+        if (closeCore) {
+          core.close();
+        }
       }
     }
     

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

@@ -354,6 +354,19 @@ public class TestLazyCores extends SolrTestCaseJ4 {
 
   }
 
+  private void unloadViaAdmin(CoreContainer cc, String name) throws Exception {
+
+    final CoreAdminHandler admin = new CoreAdminHandler(cc);
+    SolrQueryResponse resp = new SolrQueryResponse();
+    admin.handleRequestBody
+        (req(CoreAdminParams.ACTION,
+            CoreAdminParams.CoreAdminAction.UNLOAD.toString(),
+            CoreAdminParams.CORE, name),
+            resp);
+
+  }
+
+
   // Make sure that creating a transient core from the admin handler correctly respects the transient limits etc.
   @Test
   public void testCreateTransientFromAdmin() throws Exception {
@@ -382,6 +395,15 @@ public class TestLazyCores extends SolrTestCaseJ4 {
 
       checkInCores(cc, "collection1", "collectionLazy5", "core2", "core3", "core4", "core5");
 
+      // While we're at it, a test for SOLR-5366, unloading transient core that's been unloaded b/c it's
+      // transient generates a "too many closes" errorl
+
+      unloadViaAdmin(cc, "core1");
+      unloadViaAdmin(cc, "core2");
+      unloadViaAdmin(cc, "core3");
+      unloadViaAdmin(cc, "core4");
+      unloadViaAdmin(cc, "core5");
+
       c1.close();
       c2.close();
       c3.close();
@@ -393,6 +415,7 @@ public class TestLazyCores extends SolrTestCaseJ4 {
     }
   }
 
+
   //Make sure persisting not-loaded lazy cores is done. See SOLR-4347
 
   @Test