You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2013/04/18 02:45:25 UTC

svn commit: r1469112 - in /lucene/dev/branches/branch_4x: ./ solr/ solr/core/ solr/core/src/java/org/apache/solr/core/ solr/core/src/test/org/apache/solr/core/

Author: markrmiller
Date: Thu Apr 18 00:45:24 2013
New Revision: 1469112

URL: http://svn.apache.org/r1469112
Log:
SOLR-4662: Discover SolrCores by directory structure rather than defining them in solr.xml. Also, change the format of solr.xml to be closer to that of solrconfig.xml.  This version of Solr will ship the example in the old style, but you can manually try the new style. Solr 4.4 will ship with the new style, and Solr 5.0 will remove support for the old style.

Modified:
    lucene/dev/branches/branch_4x/   (props changed)
    lucene/dev/branches/branch_4x/solr/   (props changed)
    lucene/dev/branches/branch_4x/solr/CHANGES.txt   (contents, props changed)
    lucene/dev/branches/branch_4x/solr/build.xml   (contents, props changed)
    lucene/dev/branches/branch_4x/solr/core/   (props changed)
    lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolr.java
    lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java
    lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreContainer.java
    lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java
    lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
    lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java

Modified: lucene/dev/branches/branch_4x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/CHANGES.txt?rev=1469112&r1=1469111&r2=1469112&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/solr/CHANGES.txt Thu Apr 18 00:45:24 2013
@@ -120,26 +120,18 @@ New Features
 * SOLR-4530: DIH: Provide configuration to use Tika's IdentityHtmlMapper
   (Alexandre Rafalovitch via shalin)
   
-* SOLR-4663: Log an error if more than one core has the same name or points to the same
-  data directory. The new discovery-based core enumeration will be particularly sensitive
-  to this. (Erick Erickson)
-  
-* SOLR-4347: Insure that newly-created cores via Admin handler are persisted in solr.xml
+* SOLR-4662: Discover SolrCores by directory structure rather than defining them
+  in solr.xml. Also, change the format of solr.xml to be closer to that of solrconfig.xml.
+  This version of Solr will ship the example in the old style, but you can manually
+  try the new style. Solr 4.4 will ship with the new style, and Solr 5.0 will remove
+  support for the old style. (Erick Erickson, Mark Miller)
+  Additional Work:
+  - SOLR-4347: Ensure that newly-created cores via Admin handler are persisted in solr.xml
   (Erick Erickson)
-  
-* SOLR-1905: Cores created by the admin request handler should be persisted to solr.xml.
+  - SOLR-1905: Cores created by the admin request handler should be persisted to solr.xml.
   Also fixed a problem whereby properties like solr.solr.datadir would be persisted
   to solr.xml. Also, cores that didn't happen to be loaded were not persisted. 
   (Erick Erickson)
-  
-* SOLR-4662: Finalize what we're going to do with solr.xml, auto-discovery, config sets.
-  The format of solr.xml is changing. <core> and <cores> tags are no longer supported as
-  of 5.0 (but will be supported for 4.x). defaultCoreName, persistent and adminPath are
-  obsolete in 5.0 as well. Discovery mode will be the way cores are enumerated in 5.0.
-  Supports a new value (<cores> attribute coreRootDirectory in 4.x, element in <solr> in
-  5.0) for basing the enumeration of cores. In the new way of doing things, a
-  core.propeties file will mark the instanceDir for that core, and instanceDir will
-  be obsolete as well
 
 * SOLR-4717/SOLR-1351: SimpleFacets now work with localParams allowing faceting on the 
   same field multiple ways (ryan, Uri Boness)

Modified: lucene/dev/branches/branch_4x/solr/build.xml
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/build.xml?rev=1469112&r1=1469111&r2=1469112&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/build.xml (original)
+++ lucene/dev/branches/branch_4x/solr/build.xml Thu Apr 18 00:45:24 2013
@@ -59,7 +59,6 @@
   <target name="run-example" depends="example"
           description="Run Solr interactively, via Jetty.  -Dexample.debug=true to enable JVM debugger">
     <property name="example.solr.home" location="example/solr"/>
-    <property name="example.data.dir" location="example/solr/data"/>
     <property name="example.debug.suspend" value="n"/>
     <property name="example.jetty.port" value="8983"/>
     <condition property="example.jvm.line" value="-Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=${example.debug.suspend},address=5005">
@@ -70,7 +69,6 @@
     <java jar="${example}/start.jar" fork="true" dir="${example}" maxmemory="${example.heap.size}">
       <jvmarg line="${example.jvm.line}"/>
       <sysproperty key="solr.solr.home" file="${example.solr.home}"/>
-      <sysproperty key="solr.data.dir" value="${example.data.dir}"/>
       <sysproperty key="jetty.port" value="${example.jetty.port}"/>
     </java>
   </target>

Modified: lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolr.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolr.java?rev=1469112&r1=1469111&r2=1469112&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolr.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolr.java Thu Apr 18 00:45:24 2013
@@ -84,7 +84,7 @@ public interface ConfigSolr {
 
   public ShardHandlerFactory initShardHandler();
 
-  public Properties getSolrProperties(ConfigSolr cfg, String context);
+  public Properties getSolrProperties(String context);
 
   public SolrConfig getSolrConfigFromZk(ZkController zkController, String zkConfigName, String solrConfigFileName,
                                         SolrResourceLoader resourceLoader);
@@ -108,7 +108,7 @@ public interface ConfigSolr {
 
   // If the core is not to be loaded (say two cores defined with the same name or with the same data dir), return
   // the reason. If it's OK to load the core, return null.
-  public String getBadCoreMessage(String name);
+  public String getBadConfigCoreMessage(String name);
 
   public boolean is50OrLater();
 }

Modified: lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java?rev=1469112&r1=1469111&r2=1469112&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java Thu Apr 18 00:45:24 2013
@@ -80,7 +80,7 @@ public class ConfigSolrXml extends Confi
 
   private final Map<String, CoreDescriptorPlus> coreDescriptorPlusMap = new HashMap<String, CoreDescriptorPlus>();
   private NodeList coreNodes = null;
-  private final Map<String, String> badCores = new HashMap<String, String>();
+  private final Map<String, String> badConfigCores = new HashMap<String, String>();
     // List of cores that we should _never_ load. Ones with dup names or duplicate datadirs or...
 
 
@@ -101,6 +101,7 @@ public class ConfigSolrXml extends Confi
     super(loader, null, copyDoc(cfg.getDocument())); // Mimics a call from CoreContainer.
     init(container);
   }
+  
   private void init(CoreContainer container) throws IOException {
     is50OrLater = getNode("solr/cores", false) == null;
 
@@ -108,61 +109,61 @@ public class ConfigSolrXml extends Confi
     // TODO: 5.0 maybe remove this checking, it's mostly for correctness as we make this transition.
 
     if (is50OrLater()) {
-      insureFail("solr/@coreLoadThreads");
-      insureFail("solr/@persist");
-      insureFail("solr/@sharedLib");
-      insureFail("solr/@zkHost");
-
-      insureFail("solr/logging/@class");
-      insureFail("solr/logging/@enabled");
-      insureFail("solr/logging/watcher/@size");
-      insureFail("solr/logging/watcher/@threshold");
-
-      insureFail("solr/cores/@adminHandler");
-      insureFail("solr/cores/@distribUpdateConnTimeout");
-      insureFail("solr/cores/@distribUpdateSoTimeout");
-      insureFail("solr/cores/@host");
-      insureFail("solr/cores/@hostContext");
-      insureFail("solr/cores/@hostPort");
-      insureFail("solr/cores/@leaderVoteWait");
-      insureFail("solr/cores/@managementPath");
-      insureFail("solr/cores/@shareSchema");
-      insureFail("solr/cores/@transientCacheSize");
-      insureFail("solr/cores/@zkClientTimeout");
+      failIfFound("solr/@coreLoadThreads");
+      failIfFound("solr/@persist");
+      failIfFound("solr/@sharedLib");
+      failIfFound("solr/@zkHost");
+
+      failIfFound("solr/logging/@class");
+      failIfFound("solr/logging/@enabled");
+      failIfFound("solr/logging/watcher/@size");
+      failIfFound("solr/logging/watcher/@threshold");
+
+      failIfFound("solr/cores/@adminHandler");
+      failIfFound("solr/cores/@distribUpdateConnTimeout");
+      failIfFound("solr/cores/@distribUpdateSoTimeout");
+      failIfFound("solr/cores/@host");
+      failIfFound("solr/cores/@hostContext");
+      failIfFound("solr/cores/@hostPort");
+      failIfFound("solr/cores/@leaderVoteWait");
+      failIfFound("solr/cores/@managementPath");
+      failIfFound("solr/cores/@shareSchema");
+      failIfFound("solr/cores/@transientCacheSize");
+      failIfFound("solr/cores/@zkClientTimeout");
 
-      // These have no counterpart in 5.0, asking for any o fthese in Solr 5.0 will result in an error being
+      // These have no counterpart in 5.0, asking for any of these in Solr 5.0 will result in an error being
       // thrown.
-      insureFail("solr/cores/@defaultCoreName");
-      insureFail("solr/@persistent");
-      insureFail("solr/cores/@adminPath");
+      failIfFound("solr/cores/@defaultCoreName");
+      failIfFound("solr/@persistent");
+      failIfFound("solr/cores/@adminPath");
     } else {
-      insureFail("solr/str[@name='adminHandler']");
-      insureFail("solr/int[@name='coreLoadThreads']");
-      insureFail("solr/str[@name='coreRootDirectory']");
-      insureFail("solr/solrcloud/int[@name='distribUpdateConnTimeout']");
-      insureFail("solr/solrcloud/int[@name='distribUpdateSoTimeout']");
-      insureFail("solr/solrcloud/str[@name='host']");
-      insureFail("solr/solrcloud/str[@name='hostContext']");
-      insureFail("solr/solrcloud/int[@name='hostPort']");
-      insureFail("solr/solrcloud/int[@name='leaderVoteWait']");
-      insureFail("solr/str[@name='managementPath']");
-      insureFail("solr/str[@name='sharedLib']");
-      insureFail("solr/str[@name='shareSchema']");
-      insureFail("solr/int[@name='transientCacheSize']");
-      insureFail("solr/solrcloud/int[@name='zkClientTimeout']");
-      insureFail("solr/solrcloud/int[@name='zkHost']");
+      failIfFound("solr/str[@name='adminHandler']");
+      failIfFound("solr/int[@name='coreLoadThreads']");
+      failIfFound("solr/str[@name='coreRootDirectory']");
+      failIfFound("solr/solrcloud/int[@name='distribUpdateConnTimeout']");
+      failIfFound("solr/solrcloud/int[@name='distribUpdateSoTimeout']");
+      failIfFound("solr/solrcloud/str[@name='host']");
+      failIfFound("solr/solrcloud/str[@name='hostContext']");
+      failIfFound("solr/solrcloud/int[@name='hostPort']");
+      failIfFound("solr/solrcloud/int[@name='leaderVoteWait']");
+      failIfFound("solr/str[@name='managementPath']");
+      failIfFound("solr/str[@name='sharedLib']");
+      failIfFound("solr/str[@name='shareSchema']");
+      failIfFound("solr/int[@name='transientCacheSize']");
+      failIfFound("solr/solrcloud/int[@name='zkClientTimeout']");
+      failIfFound("solr/solrcloud/int[@name='zkHost']");
 
-      insureFail("solr/logging/str[@name='class']");
-      insureFail("solr/logging/str[@name='enabled']");
+      failIfFound("solr/logging/str[@name='class']");
+      failIfFound("solr/logging/str[@name='enabled']");
 
-      insureFail("solr/logging/watcher/int[@name='size']");
-      insureFail("solr/logging/watcher/int[@name='threshold']");
+      failIfFound("solr/logging/watcher/int[@name='size']");
+      failIfFound("solr/logging/watcher/int[@name='threshold']");
 
     }
     fillPropMap();
     initCoreList(container);
   }
-  private void insureFail(String xPath) {
+  private void failIfFound(String xPath) {
 
     if (getVal(xPath, false) != null) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Should not have found " + xPath +
@@ -178,6 +179,7 @@ public class ConfigSolrXml extends Confi
     }
     return val;
   }
+  
   private void fillPropMap() {
     if (is50OrLater) { // Can do the prop subs early here since we don't need to preserve them for persistence.
       propMap.put(CfgProp.SOLR_ADMINHANDLER, doSub("solr/str[@name='adminHandler']"));
@@ -231,7 +233,7 @@ public class ConfigSolrXml extends Confi
       propMap.put(CfgProp.SOLR_SHARDHANDLERFACTORY_CONNTIMEOUT, getVal(  "solr/shardHandlerFactory/int[@connTimeout]", false));
       propMap.put(CfgProp.SOLR_SHARDHANDLERFACTORY_SOCKETTIMEOUT, getVal("solr/shardHandlerFactory/int[@socketTimeout]", false));
 
-      // These have no counterpart in 5.0, asking, for any o, fthese in Solr 5.0 will result in an error being
+      // These have no counterpart in 5.0, asking, for any of these in Solr 5.0 will result in an error being
       // thrown.
       propMap.put(CfgProp.SOLR_CORES_DEFAULT_CORE_NAME, getVal("solr/cores/@defaultCoreName", false));
       propMap.put(CfgProp.SOLR_PERSISTENT, getVal("solr/@persistent", false));
@@ -239,8 +241,7 @@ public class ConfigSolrXml extends Confi
     }
   }
 
-  //NOTE:
-  public void initCoreList(CoreContainer container) throws IOException {
+  private void initCoreList(CoreContainer container) throws IOException {
     if (is50OrLater) {
       if (container != null) { //TODO: 5.0. Yet another bit of nonsense only because of the test harness.
         synchronized (coreDescriptorPlusMap) {
@@ -265,7 +266,6 @@ public class ConfigSolrXml extends Confi
           } else {
             String msg = String.format(Locale.ROOT, "More than one core defined for core named %s", name);
             log.error(msg);
-            badCores.put(name, msg);
           }
         }
 
@@ -275,8 +275,7 @@ public class ConfigSolrXml extends Confi
           } else {
             String msg = String.format(Locale.ROOT, "More than one core points to data dir %s. They are in %s and %s",
                 dataDir, dirs.get(dataDir), name);
-            log.error(msg);
-            badCores.put(name, msg);
+            log.warn(msg);
           }
         }
       }
@@ -284,9 +283,10 @@ public class ConfigSolrXml extends Confi
   }
 
   @Override
-  public String getBadCoreMessage(String name) {
-    return badCores.get(name);
+  public String getBadConfigCoreMessage(String name) {
+    return badConfigCores.get(name);
   }
+  
   public static Document copyDoc(Document doc) throws TransformerException {
     TransformerFactory tfactory = TransformerFactory.newInstance();
     Transformer tx = tfactory.newTransformer();
@@ -353,10 +353,10 @@ public class ConfigSolrXml extends Confi
   }
 
   @Override
-  public Properties getSolrProperties(ConfigSolr cfg, String context) {
+  public Properties getSolrProperties(String path) {
     try {
       return readProperties(((NodeList) evaluate(
-          context, XPathConstants.NODESET)).item(0));
+          path, XPathConstants.NODESET)).item(0));
     } catch (Throwable e) {
       SolrException.log(log, null, e);
     }
@@ -409,9 +409,9 @@ public class ConfigSolrXml extends Confi
   // deeper in the tree.
   //
   // @param file - the directory we're to either read the properties file from or recurse into.
-  private void walkFromHere(File file, CoreContainer container, Map<String, String>seenDirs, HashMap<String, String> seenCores)
+  private void walkFromHere(File file, CoreContainer container, Map<String, String> seenDirs, HashMap<String, String> seenCores)
       throws IOException {
-    log.info("Looking for cores in " + file.getAbsolutePath());
+    log.info("Looking for cores in " + file.getCanonicalPath());
     if (! file.exists()) return;
 
     for (File childFile : file.listFiles()) {
@@ -471,34 +471,15 @@ public class ConfigSolrXml extends Confi
           desc.getName(), propFile.getAbsolutePath(), seenCores.get(desc.getName()));
       log.error(msg);
       // Load up as many errors as there are.
-      if (badCores.containsKey(desc.getName())) msg += " " + badCores.get(desc.getName());
-      badCores.put(desc.getName(), msg);
+      if (badConfigCores.containsKey(desc.getName())) msg += " " + badConfigCores.get(desc.getName());
+      badConfigCores.put(desc.getName(), msg);
     }
     // There's no reason both errors may not have occurred.
     if (seenDirs.containsKey(desc.getAbsoluteDataDir())) {
       String msg = String.format(Locale.ROOT, "More than one core points to data dir '%s'. They are in '%s' and '%s'. Removing all offending cores.",
           desc.getAbsoluteDataDir(), propFile.getAbsolutePath(), seenDirs.get(desc.getAbsoluteDataDir()));
-      if (badCores.containsKey(desc.getName())) msg += " " + badCores.get(desc.getName());
-      log.error(msg);
-      badCores.put(desc.getName(), msg);
-
-      // find the core with this datadir and remove it
-      List<String> badNames = new ArrayList<String>();
-      for (Map.Entry<String, CoreDescriptorPlus> ent : coreDescriptorPlusMap.entrySet()) {
-        if (ent.getValue().getCoreDescriptor().getAbsoluteDataDir().equals(desc.getAbsoluteDataDir())) {
-          badNames.add(ent.getKey());
-          if (! badCores.containsKey(ent.getKey())) {
-            // Record that the first core is also a bad core.
-            badCores.put(ent.getKey(), msg);
-            log.error(msg);
-
-          }
-          break;
-        }
-      }
-      for (String badName : badNames) {
-        coreDescriptorPlusMap.remove(badName);
-      }
+      if (badConfigCores.containsKey(desc.getName())) msg += " " + badConfigCores.get(desc.getName());
+      log.warn(msg);
     }
     coreDescriptorPlusMap.remove(desc.getName());
   }

Modified: lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreContainer.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreContainer.java?rev=1469112&r1=1469111&r2=1469112&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreContainer.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreContainer.java Thu Apr 18 00:45:24 2013
@@ -458,7 +458,9 @@ public class CoreContainer
         defaultCoreName = dcoreName;
       }
       persistent = cfg.getBool(ConfigSolr.CfgProp.SOLR_PERSISTENT, false);
-      adminPath = cfg.get(ConfigSolr.CfgProp.SOLR_ADMINPATH, null);
+      adminPath = cfg.get(ConfigSolr.CfgProp.SOLR_ADMINPATH, "/admin/cores");
+    } else {
+      adminPath = "/admin/cores";
     }
     zkHost = cfg.get(ConfigSolr.CfgProp.SOLR_ZKHOST, null);
     coreLoadThreads = cfg.getInt(ConfigSolr.CfgProp.SOLR_CORELOADTHREADS, CORE_LOAD_THREADS);
@@ -506,7 +508,7 @@ public class CoreContainer
     }
     
     collectionsHandler = new CollectionsHandler(this);
-    containerProperties = cfg.getSolrProperties(cfg, DEFAULT_HOST_CONTEXT);
+    containerProperties = cfg.getSolrProperties("solr");
 
     // setup executor to load cores in parallel
     coreLoadExecutor = new ThreadPoolExecutor(coreLoadThreads, coreLoadThreads, 1,
@@ -662,7 +664,7 @@ public class CoreContainer
 
   private volatile boolean isShutDown = false;
 
-  private volatile ConfigSolr cfg;
+  volatile ConfigSolr cfg;
   
   public boolean isShutDown() {
     return isShutDown;
@@ -702,7 +704,10 @@ public class CoreContainer
         try {
           backgroundCloser.join();
         } catch (InterruptedException e) {
-          ; // Don't much care if this gets interrupted
+          Thread.currentThread().interrupt();
+          if (log.isDebugEnabled()) {
+            log.debug("backgroundCloser thread was interrupted before finishing");
+          }
         }
       }
       // Now clear all the cores that are being operated upon.
@@ -910,7 +915,7 @@ public class CoreContainer
     try {
       config = new SolrConfig(solrLoader, dcore.getConfigName(), null);
     } catch (Exception e) {
-      log.error("Failed to load file {}/{}", instanceDir, dcore.getConfigName());
+      log.error("Failed to load file {}", new File(instanceDir, dcore.getConfigName()).getAbsolutePath());
       throw new SolrException(ErrorCode.SERVER_ERROR, "Could not load config for " + dcore.getConfigName(), e);
     }
 
@@ -958,6 +963,10 @@ public class CoreContainer
    */
   public SolrCore create(CoreDescriptor dcore) {
 
+    if (isShutDown) {
+      throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE, "Solr has shutdown.");
+    }
+    
     final String name = dcore.getName();
 
     try {
@@ -1064,7 +1073,7 @@ public class CoreContainer
       name = checkDefault(name);
 
       if (cfg != null) { // Another test artifact.
-        String badMsg = cfg.getBadCoreMessage(name);
+        String badMsg = cfg.getBadConfigCoreMessage(name);
         if (badMsg != null) {
           throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, badMsg);
         }
@@ -1178,7 +1187,7 @@ public class CoreContainer
     name = checkDefault(name);
 
     if (cfg != null) { // Get this out of here sometime, this is test-code only stuff!
-      String badMsg = cfg.getBadCoreMessage(name);
+      String badMsg = cfg.getBadConfigCoreMessage(name);
       if (badMsg != null) {
         throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, badMsg);
       }
@@ -1284,12 +1293,6 @@ public class CoreContainer
     return zkClientTimeout;
   }
 
-
-  public void setAdminPath(String adminPath) {
-      this.adminPath = adminPath;
-  }
-  
-
   public String getManagementPath() {
     return managementPath;
   }
@@ -1451,7 +1454,7 @@ public class CoreContainer
   }
 
   public String getBadCoreMessage(String name) {
-    return cfg.getBadCoreMessage(name);
+    return cfg.getBadConfigCoreMessage(name);
   }
 
 }
@@ -1930,7 +1933,7 @@ class CoreMaps {
     Properties persistProps = new Properties();
     CloudDescriptor cd = dcore.getCloudDescriptor();
     String collection = null;
-    if (cd  != null) collection = cd.getCollectionName();
+    if (cd != null) collection = cd.getCollectionName();
     String instDir = dcore.getRawInstanceDir();
 
     if (cfg == null) {
@@ -1969,13 +1972,17 @@ class CoreMaps {
 
       coreAttribs = cfg.readCoreAttributes(origCoreName);
       persistProps = cfg.readCoreProperties(origCoreName);
-      if (coreAttribs != null) {
-        coreAttribs.put(CoreDescriptor.CORE_NAME, coreName);
-        if (coreAttribs.containsKey(CoreDescriptor.CORE_COLLECTION)) collection = coreAttribs.get(CoreDescriptor.CORE_COLLECTION);
-        if (coreAttribs.containsKey(CoreDescriptor.CORE_INSTDIR)) instDir = coreAttribs.get(CoreDescriptor.CORE_INSTDIR);
-      }
-      addIfNotNull(coreAttribs, CoreDescriptor.CORE_INSTDIR, dcore.getRawInstanceDir());
-      coreAttribs.put(CoreDescriptor.CORE_COLLECTION, StringUtils.isNotBlank(collection) ? collection : dcore.getName());
+      
+      coreAttribs.put(CoreDescriptor.CORE_NAME, coreName);
+      if (coreAttribs.containsKey(CoreDescriptor.CORE_COLLECTION)) collection = coreAttribs
+          .get(CoreDescriptor.CORE_COLLECTION);
+      if (coreAttribs.containsKey(CoreDescriptor.CORE_INSTDIR)) instDir = coreAttribs
+          .get(CoreDescriptor.CORE_INSTDIR);
+      
+      addIfNotNull(coreAttribs, CoreDescriptor.CORE_INSTDIR,
+          dcore.getRawInstanceDir());
+      coreAttribs.put(CoreDescriptor.CORE_COLLECTION,
+          StringUtils.isNotBlank(collection) ? collection : dcore.getName());
 
     }
 

Modified: lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java?rev=1469112&r1=1469111&r2=1469112&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java Thu Apr 18 00:45:24 2013
@@ -22,6 +22,7 @@ import java.io.File;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.solr.cloud.CloudDescriptor;
+import org.apache.solr.core.ConfigSolr.CfgProp;
 
 /**
  * A Solr core descriptor
@@ -33,7 +34,7 @@ public class CoreDescriptor {
   // Properties file name constants
   public static final String CORE_NAME = "name";
   public static final String CORE_CONFIG = "config";
-  public static final String CORE_INSTDIR = "instanceDir";
+  public static final String CORE_INSTDIR = "instanceDir"; // should probably be removed after 4x
   public static final String CORE_DATADIR = "dataDir";
   public static final String CORE_ULOGDIR = "ulogDir";
   public static final String CORE_SCHEMA = "schema";
@@ -209,7 +210,14 @@ public class CoreDescriptor {
     }
 
     if (coreContainer == null) return null;
-
+    if( coreContainer.cfg != null) {
+      String coreRootDir = coreContainer.cfg.get(
+          CfgProp.SOLR_COREROOTDIRECTORY, null);
+      if (coreRootDir != null) {
+        return SolrResourceLoader.normalizeDir(coreRootDir
+            + SolrResourceLoader.normalizeDir(instDir));
+      }
+    }
     return SolrResourceLoader.normalizeDir(coreContainer.getSolrHome() +
         SolrResourceLoader.normalizeDir(instDir));
   }

Modified: lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java?rev=1469112&r1=1469111&r2=1469112&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java Thu Apr 18 00:45:24 2013
@@ -297,65 +297,6 @@ public class TestCoreContainer extends S
 
   }
 
-  @Test
-  public void testCoresSameName() throws IOException, ParserConfigurationException, SAXException {
-    //create solrHome
-    File solrHomeDirectory = new File(TEMP_DIR, this.getClass().getName()
-        + "_sameName");
-    SetUpHome(solrHomeDirectory, SOLR_XML_SAME_NAME);
-    copyMinConf(new File(solrHomeDirectory, "core1"));
-    copyMinConf(new File(solrHomeDirectory, "core2"));
-    CoreContainer.Initializer init = new CoreContainer.Initializer();
-    CoreContainer cores = null;
-    SolrCore core1 = null;
-    try {
-      cores = init.initialize();
-      core1 = cores.getCore("core1");
-    } catch(SolrException se) {
-      assertEquals("Exception code should be 500", 500, se.code());
-      assertTrue("Should have seen an exception when two cores have the same name",
-          se.getMessage().contains("More than one core defined for core named"));
-
-    } finally {
-      if (cores != null) {
-        if (core1 != null) core1.close();
-        cores.shutdown();
-      }
-      FileUtils.deleteDirectory(solrHomeDirectory);
-    }
-  }
-
-  @Test
-  public void testCoresSameDataDir() throws IOException, ParserConfigurationException, SAXException {
-    //create solrHome
-    File solrHomeDirectory = new File(TEMP_DIR, this.getClass().getName()
-        + "_sameDataDir");
-    SetUpHome(solrHomeDirectory, SOLR_XML_SAME_DATADIR);
-    copyMinConf(new File(solrHomeDirectory, "core1"));
-    copyMinConf(new File(solrHomeDirectory, "core2"));
-    CoreContainer.Initializer init = new CoreContainer.Initializer();
-    CoreContainer cores = null;
-    SolrCore core1 = null;
-    SolrCore core2 = null;
-    try {
-      cores = init.initialize();
-      core1 = cores.getCore("core1");
-      core2 = cores.getCore("core2");
-    }
-    catch(SolrException se) {
-      assertEquals("Exception code should be 500", 500, se.code());
-      assertTrue("Should have seen an exception when two cores have the same data dir",
-          se.getMessage().contains("More than one core points to data dir"));
-    } finally {
-      if (cores != null) {
-        if (core1 != null) core1.close();
-        if (core2 != null) core2.close();
-        cores.shutdown();
-      }
-      FileUtils.deleteDirectory(solrHomeDirectory);
-    }
-  }
-
   private void SetUpHome(File solrHomeDirectory, String xmlFile) throws IOException {
     if (solrHomeDirectory.exists()) {
       FileUtils.deleteDirectory(solrHomeDirectory);

Modified: lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java?rev=1469112&r1=1469111&r2=1469112&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java Thu Apr 18 00:45:24 2013
@@ -159,7 +159,6 @@ public class TestCoreDiscovery extends S
 
     CoreContainer cc = init();
     try {
-      assertNull("adminPath no longer allowed in solr.xml", cc.getAdminPath());
       assertNull("defaultCore no longer allowed in solr.xml", cc.getDefaultCoreName());
 
       assertEquals("222.333.444.555", cc.getHost());
@@ -232,14 +231,12 @@ public class TestCoreDiscovery extends S
       cc = init();
       String msg = cc.getBadCoreMessage("core1");
       assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
       try {
         cc.getCore("core1");
       } catch (SolrException se) {
         assertEquals("Should be returning proper error code of 500", 500, se.code());
         msg = se.getMessage();
         assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
       }
     } finally {
       if (cc != null) {
@@ -249,41 +246,6 @@ public class TestCoreDiscovery extends S
   }
 
   @Test
-  public void testCoresWithSameDataDirError() throws Exception {
-    setMeUp();
-    addCoreWithProps(makeCorePropFile("core1", false, true, "dataDir=" + solrHomeDirectory + "datadir"));
-    addCoreWithProps(makeCorePropFile("core2", false, true, "dataDir=" + solrHomeDirectory + "datadir"));
-    CoreContainer cc = null;
-    try {
-      cc = init();
-      String msg = cc.getBadCoreMessage("core2");
-      assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
-      try {
-        cc.getCore("core1");
-      } catch (SolrException se) {
-        assertEquals("Should be returning proper error code of 500", 500, se.code());
-        msg = se.getMessage();
-        assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
-      }
-      try {
-        cc.getCore("core2");
-      } catch (SolrException se) {
-        assertEquals("Should be returning proper error code of 500", 500, se.code());
-        msg = se.getMessage();
-        assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
-      }
-
-    } finally {
-      if (cc != null) {
-        cc.shutdown();
-      }
-    }
-  }
-
-  @Test
   public void testCoresWithSameNameErrorTransient() throws Exception {
     setMeUp();
     addCoreWithPropsDir("core1_1", makeCorePropFile("core1", true, false));
@@ -293,41 +255,12 @@ public class TestCoreDiscovery extends S
       cc = init();
       String msg = cc.getBadCoreMessage("core1");
       assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
       try {
         cc.getCore("core1");
       } catch (SolrException se) {
         assertEquals("Should be returning proper error code of 500", 500, se.code());
         msg = se.getMessage();
         assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
-      }
-    } finally {
-      if (cc != null) {
-        cc.shutdown();
-      }
-    }
-  }
-
-  @Test
-  public void testCoresWithSameDataDirErrorTransient() throws Exception {
-    setMeUp();
-    addCoreWithProps(makeCorePropFile("core1", true, false, "dataDir=" + solrHomeDirectory + "datadir"));
-    addCoreWithProps(makeCorePropFile("core2", true, false, "dataDir=" + solrHomeDirectory + "datadir"));
-    // Should just blow up here.
-    CoreContainer cc = null;
-    try {
-      cc = init();
-      String msg = cc.getBadCoreMessage("core1");
-      assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
-      try {
-        cc.getCore("core1");
-      } catch (SolrException se) {
-        assertEquals("Should be returning proper error code of 500", 500, se.code());
-        msg = se.getMessage();
-        assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
       }
     } finally {
       if (cc != null) {
@@ -336,7 +269,6 @@ public class TestCoreDiscovery extends S
     }
   }
 
-
   @Test
   public void testCoresWithSameNameErrorBoth() throws Exception {
     setMeUp();
@@ -348,57 +280,13 @@ public class TestCoreDiscovery extends S
       cc = init();
       String msg = cc.getBadCoreMessage("core1");
       assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
       try {
         cc.getCore("core1");
       } catch (SolrException se) {
         assertEquals("Should be returning proper error code of 500", 500, se.code());
         msg = se.getMessage();
         assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
-      }
-    } finally {
-      if (cc != null) {
-        cc.shutdown();
-      }
-    }
-  }
-
-
-  @Test
-  public void testCoresWithSameDataDirErrorBoth() throws Exception {
-    setMeUp();
-    addCoreWithProps(makeCorePropFile("core1", false, false, "dataDir=" + solrHomeDirectory + "/datadir"));
-    addCoreWithProps(makeCorePropFile("core2", true, false, "dataDir=" + solrHomeDirectory + "/datadir"));
-    // Should just blow up here.
-    CoreContainer cc = null;
-    try {
-      cc = init();
-      String msg = cc.getBadCoreMessage("core2");
-      assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
-
-      msg = cc.getBadCoreMessage("core1");
-      assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
-
-      try {
-        cc.getCore("core1");
-      } catch (SolrException se) {
-        assertEquals("Should be returning proper error code of 500", 500, se.code());
-        msg = se.getMessage();
-        assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
       }
-      try {
-        cc.getCore("core2");
-      } catch (SolrException se) {
-        assertEquals("Should be returning proper error code of 500", 500, se.code());
-        msg = se.getMessage();
-        assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
-        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
-      }
-
     } finally {
       if (cc != null) {
         cc.shutdown();