You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by er...@apache.org on 2013/01/16 01:48:09 UTC
svn commit: r1433778 - in /lucene/dev/trunk/solr/core/src:
java/org/apache/solr/core/CoreContainer.java
test/org/apache/solr/core/TestLazyCores.java
Author: erick
Date: Wed Jan 16 00:48:09 2013
New Revision: 1433778
URL: http://svn.apache.org/viewvc?rev=1433778&view=rev
Log:
Fix for SOLR-4300, possible race condition when loading lazy cores
Modified:
lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestLazyCores.java
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java?rev=1433778&r1=1433777&r2=1433778&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java Wed Jan 16 00:48:09 2013
@@ -128,6 +128,8 @@ public class CoreContainer
protected final Map<String, CoreDescriptor> dynamicDescriptors = new LinkedHashMap<String, CoreDescriptor>();
+ protected final Set<String> pendingDynamicCoreLoads = new HashSet<String>();
+
protected final Map<String,Exception> coreInitFailures =
Collections.synchronizedMap(new LinkedHashMap<String,Exception>());
@@ -1245,17 +1247,8 @@ public class CoreContainer
}
}
}
-
- /** Gets a core by name and increase its refcount.
- * @see SolrCore#close()
- * @param name the core name
- * @return the core if found
- */
- public SolrCore getCore(String name) {
- name = checkDefault(name);
- // Do this in two phases since we don't want to lock access to the cores over a load.
+ private SolrCore getCoreFromAnyList(String name) {
SolrCore core;
-
synchronized (cores) {
core = cores.get(name);
if (core != null) {
@@ -1274,20 +1267,64 @@ public class CoreContainer
return core;
}
}
+ return null;
+ }
+ /** Gets a core by name and increase its refcount.
+ * @see SolrCore#close()
+ * @param name the core name
+ * @return the core if found
+ */
+ public SolrCore getCore(String name) {
+ name = checkDefault(name);
+ // Do this in two phases since we don't want to lock access to the cores over a load.
+ SolrCore core = getCoreFromAnyList(name);
+
+ if (core != null) return core;
+
+ // OK, it's not presently in any list, is it in the list of dynamic cores but not loaded yet? If so, load it.
CoreDescriptor desc = dynamicDescriptors.get(name);
if (desc == null) { //Nope, no transient core with this name
return null;
}
+
+ // Keep multiple threads from loading the same core at the same time.
try {
- core = create(desc); // This should throw an error if it fails.
- core.open();
- if (desc.isTransient()) {
- registerLazyCore(name, core, false); // This is a transient core
- } else {
- register(name, core, false); // This is a "permanent", although deferred-load core
+ boolean isPending;
+ synchronized (pendingDynamicCoreLoads) {
+ isPending = pendingDynamicCoreLoads.contains(name);
+ if (! isPending) {
+ pendingDynamicCoreLoads.add(name);
+ }
}
- } catch (Exception ex) {
- throw recordAndThrow(name, "Unable to create core" + name, ex);
+
+ while (isPending) {
+ try {
+ Thread.sleep(100);
+ } catch (InterruptedException e) {
+ return null; // Seems best not to do anything at all if the thread is interrupted
+ }
+
+ synchronized (pendingDynamicCoreLoads) {
+ if (!pendingDynamicCoreLoads.contains(name)) {
+ // NOTE: If, for some reason, the load failed, we'll return null here and presumably the log will show
+ // why. We'll fail all over again next time if the problem isn't corrected.
+ return getCoreFromAnyList(name);
+ }
+ }
+ }
+ try {
+ core = create(desc); // This should throw an error if it fails.
+ core.open();
+ if (desc.isTransient()) {
+ registerLazyCore(name, core, false); // This is a transient core
+ } else {
+ register(name, core, false); // This is a "permanent", although deferred-load core
+ }
+ } catch (Exception ex) {
+ throw recordAndThrow(name, "Unable to create core" + name, ex);
+ }
+ } finally {
+ pendingDynamicCoreLoads.remove(name);
}
return core;
}
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestLazyCores.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestLazyCores.java?rev=1433778&r1=1433777&r2=1433778&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestLazyCores.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestLazyCores.java Wed Jan 16 00:48:09 2013
@@ -34,8 +34,10 @@ import org.junit.Test;
import java.io.File;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
+import java.util.List;
public class TestLazyCores extends SolrTestCaseJ4 {
@@ -246,6 +248,44 @@ public class TestLazyCores extends SolrT
}
}
+ static List<SolrCore> _theCores = new ArrayList<SolrCore>();
+ // Test case for SOLR-4300
+ @Test
+ public void testRace() throws Exception {
+ final CoreContainer cc = init();
+ try {
+
+ Thread[] threads = new Thread[15];
+ for (int idx = 0; idx < threads.length; idx++) {
+ threads[idx] = new Thread() {
+ @Override
+ public void run() {
+ SolrCore core = cc.getCore("collectionLazy3");
+ synchronized (_theCores) {
+ _theCores.add(core);
+ }
+ }
+ };
+ threads[idx].start();
+ }
+
+ for (Thread thread : threads) {
+ thread.join();
+ }
+
+ for (int idx = 0; idx < _theCores.size() - 1; ++idx) {
+ assertEquals("Cores should be the same!", _theCores.get(idx), _theCores.get(idx + 1));
+ }
+
+ for (SolrCore core : _theCores) {
+ core.close();
+ }
+
+ } finally {
+ cc.shutdown();
+ }
+ }
+
private void checkNotInCores(CoreContainer cc, String... nameCheck) {
Collection<String> names = cc.getCoreNames();
for (String name : nameCheck) {