lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sar...@apache.org
Subject lucene-solr:branch_6_0: SOLR-9116: Race condition causing occasional SolrIndexSearcher leak when SolrCore is reloaded (cherry picked from commit d6a5c5a)
Date Fri, 20 May 2016 22:57:17 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/branch_6_0 3e19c5ad1 -> 5c8594042


SOLR-9116: Race condition causing occasional SolrIndexSearcher leak when SolrCore is reloaded
(cherry picked from commit d6a5c5a)


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/5c859404
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/5c859404
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/5c859404

Branch: refs/heads/branch_6_0
Commit: 5c859404289bfb0c9a5d7d1df2c07aabf61865c4
Parents: 3e19c5a
Author: Shalin Shekhar Mangar <shalin@apache.org>
Authored: Wed May 18 20:15:52 2016 +0530
Committer: Steve Rowe <sarowe@apache.org>
Committed: Fri May 20 18:56:42 2016 -0400

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  3 +
 .../src/java/org/apache/solr/core/SolrCore.java | 20 +++++++
 .../test/org/apache/solr/core/SolrCoreTest.java | 61 +++++++++++++++++++-
 3 files changed, 83 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/5c859404/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a36754d..6a41d3f 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -81,6 +81,9 @@ Bug Fixes
 
 * SOLR-9117: The first SolrCore is leaked after reload. (Jessica Cheng via shalin)
 
+* SOLR-9116: Race condition causing occasional SolrIndexSearcher leak when SolrCore is reloaded.
+  (Jessica Cheng Mallet via shalin)
+
 Other Changes
 ----------------------
 * SOLR-7516: Improve javadocs for JavaBinCodec, ObjectResolver and enforce the single-usage
policy.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/5c859404/solr/core/src/java/org/apache/solr/core/SolrCore.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java
index 7a65a72..77ded27 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -1415,6 +1415,18 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
   private RefCounted<SolrIndexSearcher> realtimeSearcher;
   private Callable<DirectoryReader> newReaderCreator;
 
+  // For testing
+  boolean areAllSearcherReferencesEmpty() {
+    boolean isEmpty;
+    synchronized (searcherLock) {
+      isEmpty = _searchers.isEmpty();
+      isEmpty = isEmpty && _realtimeSearchers.isEmpty();
+      isEmpty = isEmpty && (_searcher == null);
+      isEmpty = isEmpty && (realtimeSearcher == null);
+    }
+    return isEmpty;
+  }
+
   /**
   * Return a registered {@link RefCounted}&lt;{@link SolrIndexSearcher}&gt; with
   * the reference count incremented.  It <b>must</b> be decremented when no longer
needed.
@@ -1614,6 +1626,14 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
       newSearcher.incref();
 
       synchronized (searcherLock) {
+        // Check if the core is closed again inside the lock in case this method is racing
with a close. If the core is
+        // closed, clean up the new searcher and bail.
+        if (isClosed()) {
+          newSearcher.decref(); // once for caller since we're not returning it
+          newSearcher.decref(); // once for ourselves since it won't be "replaced"
+          throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "openNewSearcher
called on closed core");
+        }
+
         if (realtimeSearcher != null) {
           realtimeSearcher.decref();
         }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/5c859404/solr/core/src/test/org/apache/solr/core/SolrCoreTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/core/SolrCoreTest.java b/solr/core/src/test/org/apache/solr/core/SolrCoreTest.java
index 5482707..4ae8af0 100644
--- a/solr/core/src/test/org/apache/solr/core/SolrCoreTest.java
+++ b/solr/core/src/test/org/apache/solr/core/SolrCoreTest.java
@@ -17,6 +17,7 @@
 package org.apache.solr.core;
 
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.util.ExecutorUtil;
 import org.apache.solr.handler.RequestHandlerBase;
 import org.apache.solr.handler.component.QueryComponent;
@@ -24,7 +25,9 @@ import org.apache.solr.handler.component.SpellCheckComponent;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.request.SolrRequestHandler;
 import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.search.SolrIndexSearcher;
 import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.apache.solr.util.RefCounted;
 import org.apache.solr.util.plugin.SolrCoreAware;
 import org.junit.Test;
 
@@ -33,7 +36,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 
@@ -217,6 +219,63 @@ public class SolrCoreTest extends SolrTestCaseJ4 {
     assertEquals("wrong config for queryResultWindowSize", 10, solrConfig.queryResultWindowSize);
   }
 
+  /**
+   * Test that's meant to be run with many iterations to expose a leak of SolrIndexSearcher
when a core is closed
+   * due to a reload. Without the fix, this test fails with most iters=1000 runs.
+   */
+  @Test
+  public void testReloadLeak() throws Exception {
+    final ExecutorService executor =
+        ExecutorUtil.newMDCAwareFixedThreadPool(1, new DefaultSolrThreadFactory("testReloadLeak"));
+
+    // Continuously open new searcher while core is not closed, and reload core to try to
reproduce searcher leak.
+    // While in practice we never continuously open new searchers, this is trying to make
up for the fact that opening
+    // a searcher in this empty core is very fast by opening new searchers continuously to
increase the likelihood
+    // for race.
+    SolrCore core = h.getCore();
+    assertTrue("Refcount != 1", core.getOpenCount() == 1);
+    executor.execute(new NewSearcherRunnable(core));
+
+    // Since we called getCore() vs getCoreInc() and don't own a refCount, the container
should decRef the core
+    // and close it when we call reload.
+    h.reload();
+
+    executor.shutdown();
+    executor.awaitTermination(1, TimeUnit.MINUTES);
+
+    // Check that all cores are closed and no searcher references are leaked.
+    assertTrue("SolrCore " + core + " is not closed", core.isClosed());
+    assertTrue(core.areAllSearcherReferencesEmpty());
+  }
+
+  private static class NewSearcherRunnable implements Runnable {
+    private final SolrCore core;
+
+    NewSearcherRunnable(SolrCore core) {
+      this.core = core;
+    }
+
+    @Override
+    public void run() {
+      while (!core.isClosed()) {
+        try {
+          RefCounted<SolrIndexSearcher> newSearcher = null;
+          try {
+            newSearcher = core.openNewSearcher(true, true);
+          } finally {
+            if (newSearcher != null) {
+              newSearcher.decref();
+            }
+          }
+        } catch (SolrException e) {
+          if (!core.isClosed()) {
+            throw e;
+          }
+        }
+      }
+    }
+  }
+
 }
 
 


Mime
View raw message