SOLR-6279: cores?action=UNLOAD now waits for the core to close before unregistering it from ZK.

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

solr/CHANGES.txt

@@ -393,6 +393,9 @@ Bug Fixes
 * SOLR-8453: Solr should attempt to consume the request inputstream on errors as we cannot
   count on the container to do it. (Mark Miller, Greg Wilkins, yonik, Joakim Erdfelt)
 
+* SOLR-6279: cores?action=UNLOAD now waits for the core to close before unregistering it from ZK.
+  (Christine Poerschke)
+
 Optimizations
 ----------------------
 

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

@@ -995,7 +995,7 @@ public class CoreContainer {
     
     core.unloadOnClose(deleteIndexDir, deleteDataDir, deleteInstanceDir);
     if (close)
-      core.close();
+      core.closeAndWait();
 
     if (zkSys.getZkController() != null) {
       try {

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

@@ -1006,6 +1006,27 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
     }
     return updateLogDir;
   }
+
+  /**
+   * Close the core, if it is still in use waits until is no longer in use.
+   * @see #close() 
+   * @see #isClosed() 
+   */
+  public void closeAndWait() {
+    close();
+    while (!isClosed()) {
+      final long milliSleep = 100;
+      log.info("Core {} is not yet closed, waiting {} ms before checking again.", getName(), milliSleep);
+      try {
+        Thread.sleep(milliSleep);
+      } catch (InterruptedException e) {
+        Thread.currentThread().interrupt();
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+            "Caught InterruptedException whilst waiting for core " + getName() + " to close: "
+                + e.getMessage(), e);
+      }
+    }
+  }
   
   private Codec initCodec(SolrConfig solrConfig, final IndexSchema schema) {
     final PluginInfo info = solrConfig.getPluginInfo(CodecFactory.class.getName());

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

@@ -383,11 +383,11 @@ public class TestLazyCores extends SolrTestCaseJ4 {
       createViaAdmin(cc, "core4", true, false);
       createViaAdmin(cc, "core5", true, false);
 
-      SolrCore c1 = cc.getCore("core1");
-      SolrCore c2 = cc.getCore("core2");
-      SolrCore c3 = cc.getCore("core3");
-      SolrCore c4 = cc.getCore("core4");
-      SolrCore c5 = cc.getCore("core5");
+      final SolrCore c1 = cc.getCore("core1");
+      final SolrCore c2 = cc.getCore("core2");
+      final SolrCore c3 = cc.getCore("core3");
+      final SolrCore c4 = cc.getCore("core4");
+      final SolrCore c5 = cc.getCore("core5");
 
       checkNotInCores(cc, "core1", "collection2", "collection3", "collection4", "collection6"
           , "collection7", "collection8", "collection9");
@@ -397,17 +397,46 @@ public class TestLazyCores extends SolrTestCaseJ4 {
       // 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
 
+      class TestThread extends Thread {
+        
+        @Override
+        public void run() {
+          
+          final int sleep_millis = random().nextInt(1000);
+          try {
+            if (sleep_millis > 0) {
+              if (VERBOSE) {
+                System.out.println("TestLazyCores.testCreateTransientFromAdmin Thread.run sleeping for "+sleep_millis+" ms");
+              }
+              Thread.sleep(sleep_millis);
+            }
+          }
+          catch (InterruptedException ie) {
+            if (VERBOSE) {
+              System.out.println("TestLazyCores.testCreateTransientFromAdmin Thread.run caught "+ie+" whilst sleeping for "+sleep_millis+" ms");
+            }
+          }
+
+          c1.close();
+          c2.close();
+          c3.close();
+          c4.close();
+          c5.close();
+        }
+      };
+      
+      // with SOLR-6279 UNLOAD will wait for the core's reference count to have reached zero
+      // hence cN.close() need to preceed or run in parallel with unloadViaAdmin(...)
+      final TestThread cThread = new TestThread();
+      cThread.start();
+      
       unloadViaAdmin(cc, "core1");
       unloadViaAdmin(cc, "core2");
       unloadViaAdmin(cc, "core3");
       unloadViaAdmin(cc, "core4");
       unloadViaAdmin(cc, "core5");
 
-      c1.close();
-      c2.close();
-      c3.close();
-      c4.close();
-      c5.close();
+      cThread.join();
 
     } finally {
       cc.shutdown();
@@ -638,4 +667,57 @@ public class TestLazyCores extends SolrTestCaseJ4 {
     return StringUtils.join(args, File.separator);
   }
 
+  @Test
+  public void testMidUseUnload() throws Exception {
+    final int maximumSleepMillis = random().nextInt(10000); // sleep for up to 10 s
+    if (VERBOSE) {
+      System.out.println("TestLazyCores.testMidUseUnload maximumSleepMillis="+maximumSleepMillis);
+    }
+    
+    class TestThread extends Thread {
+      
+      SolrCore core_to_use = null;
+      
+      @Override
+      public void run() {
+        
+        final int sleep_millis = random().nextInt(maximumSleepMillis);
+        try {
+          if (sleep_millis > 0) {
+            if (VERBOSE) {
+              System.out.println("TestLazyCores.testMidUseUnload Thread.run sleeping for "+sleep_millis+" ms");
+            }
+            Thread.sleep(sleep_millis);
+          }
+        }
+        catch (InterruptedException ie) {
+          if (VERBOSE) {
+            System.out.println("TestLazyCores.testMidUseUnload Thread.run caught "+ie+" whilst sleeping for "+sleep_millis+" ms");
+          }
+        }
+        
+        assertFalse(core_to_use.isClosed()); // not closed since we are still using it and hold a reference
+        core_to_use.close(); // now give up our reference to the core
+      }
+    };
+
+    CoreContainer cc = init();
+    
+    try {
+      TestThread thread = new TestThread();
+      
+      thread.core_to_use = cc.getCore("collection1");
+      assertNotNull(thread.core_to_use);
+      assertFalse(thread.core_to_use.isClosed()); // freshly-in-use core is not closed
+      thread.start();
+      
+      unloadViaAdmin(cc, "collection1");
+      assertTrue(thread.core_to_use.isClosed()); // after unload-ing the core is closed
+
+      thread.join();
+    } finally {
+      cc.shutdown();
+    }
+  }
+
 }