Solr
  1. Solr
  2. SOLR-4803

fix support for DEFAULT_DEFAULT_CORE_NAME

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.3
    • Fix Version/s: 4.4, Trunk
    • Component/s: None
    • Labels:
      None

      Description

      As far as i can tell, the "new solr.xml" changes caused the following problems...

      • DEFAULT_DEFAULT_CORE_NAME is not used at all in the "new" style solr.xml, but probably should be (at least in 4x)

      See also the mailing list thread "[JENKINS] Lucene-Solr-SmokeRelease-4.x - Build # 69 - Failure" where the lack of using DEFAULT_DEFAULT_CORE_NAME causes the smoke tester to fail on both 4x and trunk because many things assume collectionless URLs will work

      1. SOLR-4803.patch
        1 kB
        Hoss Man
      2. SOLR-4803.patch
        0.6 kB
        Hoss Man
      3. SOLR-4803.patch
        0.6 kB
        Hoss Man

        Issue Links

          Activity

          Hide
          Hoss Man added a comment -

          Patch i'm currently manually testing...

          • restores use of DEFAULT_DEFAULT_CORE_NAME with old style solr.xml when 'defaultCore' is not specified.
          • uses DEFAULT_DEFAULT_CORE_NAME as unconfigurable default with new style solr.xml - the idea being that that way trivial single collection setups like the example, or people who copy the example will be easy to use, but if you want to start doing more complicated things, you should start being explicit about your core name in the URL.
          Index: solr/core/src/java/org/apache/solr/core/CoreContainer.java
          ===================================================================
          --- solr/core/src/java/org/apache/solr/core/CoreContainer.java	(revision 1480486)
          +++ solr/core/src/java/org/apache/solr/core/CoreContainer.java	(working copy)
          @@ -281,7 +281,7 @@
               logging = JulWatcher.newRegisteredLogWatcher(cfg, loader);
           
               if (cfg instanceof ConfigSolrXmlOld) { //TODO: Remove for 5.0
          -      String dcoreName = cfg.get(ConfigSolr.CfgProp.SOLR_CORES_DEFAULT_CORE_NAME, null);
          +      String dcoreName = cfg.get(ConfigSolr.CfgProp.SOLR_CORES_DEFAULT_CORE_NAME, DEFAULT_DEFAULT_CORE_NAME);
                 if (dcoreName != null && !dcoreName.isEmpty()) {
                   defaultCoreName = dcoreName;
                 }
          @@ -289,6 +289,7 @@
                 adminPath = cfg.get(ConfigSolr.CfgProp.SOLR_ADMINPATH, "/admin/cores");
               } else {
                 adminPath = "/admin/cores";
          +      defaultCoreName = DEFAULT_DEFAULT_CORE_NAME;
               }
               zkHost = cfg.get(ConfigSolr.CfgProp.SOLR_ZKHOST, null);
               coreLoadThreads = cfg.getInt(ConfigSolr.CfgProp.SOLR_CORELOADTHREADS, CORE_LOAD_THREADS);
          
          Show
          Hoss Man added a comment - Patch i'm currently manually testing... restores use of DEFAULT_DEFAULT_CORE_NAME with old style solr.xml when 'defaultCore' is not specified. uses DEFAULT_DEFAULT_CORE_NAME as unconfigurable default with new style solr.xml - the idea being that that way trivial single collection setups like the example, or people who copy the example will be easy to use, but if you want to start doing more complicated things, you should start being explicit about your core name in the URL. Index: solr/core/src/java/org/apache/solr/core/CoreContainer.java =================================================================== --- solr/core/src/java/org/apache/solr/core/CoreContainer.java (revision 1480486) +++ solr/core/src/java/org/apache/solr/core/CoreContainer.java (working copy) @@ -281,7 +281,7 @@ logging = JulWatcher.newRegisteredLogWatcher(cfg, loader); if (cfg instanceof ConfigSolrXmlOld) { //TODO: Remove for 5.0 - String dcoreName = cfg.get(ConfigSolr.CfgProp.SOLR_CORES_DEFAULT_CORE_NAME, null); + String dcoreName = cfg.get(ConfigSolr.CfgProp.SOLR_CORES_DEFAULT_CORE_NAME, DEFAULT_DEFAULT_CORE_NAME); if (dcoreName != null && !dcoreName.isEmpty()) { defaultCoreName = dcoreName; } @@ -289,6 +289,7 @@ adminPath = cfg.get(ConfigSolr.CfgProp.SOLR_ADMINPATH, "/admin/cores"); } else { adminPath = "/admin/cores"; + defaultCoreName = DEFAULT_DEFAULT_CORE_NAME; } zkHost = cfg.get(ConfigSolr.CfgProp.SOLR_ZKHOST, null); coreLoadThreads = cfg.getInt(ConfigSolr.CfgProp.SOLR_CORELOADTHREADS, CORE_LOAD_THREADS);
          Hide
          Jan Høydahl added a comment -

          +1
          Are we sensing a quick 4.3.1 here? This one and the sharedLib bug are both going to be causing failure and mailing list traffic for people upgrading.

          Show
          Jan Høydahl added a comment - +1 Are we sensing a quick 4.3.1 here? This one and the sharedLib bug are both going to be causing failure and mailing list traffic for people upgrading.
          Hide
          Hoss Man added a comment -

          Editing issue description – my memory was faulty, and this previously mentioend problem is not really an issue...

          * DEFAULT_DEFAULT_CORE_NAME is not properly supported for the "old" style solr.xml if defaultCoreName="..." is not
          specified in the solr.xml
          

          That is not actually how defaultCoreName="..." has ever worked, i was remembering incorrectly – in the old style solr.xml, if you didn't have a defaultCoreName declared, you did not have a default core name (just like "adminPath")

          I'm scaling back my patch to only use DEFAULT_DEFAULT_CORE_NAME in the case of hte "new style" config, similar to how adminPath is hardcoded to "/admin/cores". (same reasons as mentioned before: this gets the 4x and trunk example and simple single core cases working again with simple URLs)


          Jan: as mentioned, my initial impressions about the impact of this bug were wrong, it really only affects the "new style" example on the 4x & trunk branchs, and doesn't seem to really affect anyone using existing "old style" solr.xml files.

          Show
          Hoss Man added a comment - Editing issue description – my memory was faulty, and this previously mentioend problem is not really an issue... * DEFAULT_DEFAULT_CORE_NAME is not properly supported for the "old" style solr.xml if defaultCoreName="..." is not specified in the solr.xml That is not actually how defaultCoreName="..." has ever worked, i was remembering incorrectly – in the old style solr.xml, if you didn't have a defaultCoreName declared, you did not have a default core name (just like "adminPath") I'm scaling back my patch to only use DEFAULT_DEFAULT_CORE_NAME in the case of hte "new style" config, similar to how adminPath is hardcoded to "/admin/cores". (same reasons as mentioned before: this gets the 4x and trunk example and simple single core cases working again with simple URLs) Jan: as mentioned, my initial impressions about the impact of this bug were wrong, it really only affects the "new style" example on the 4x & trunk branchs, and doesn't seem to really affect anyone using existing "old style" solr.xml files.
          Hide
          Hoss Man added a comment -

          previously mentioned (simplified) patch that i intended to commit soon.

          Show
          Hoss Man added a comment - previously mentioned (simplified) patch that i intended to commit soon.
          Hide
          Jack Krupansky added a comment -

          This is a branch_4x issue, but not a 4.3 issue - if it's the same problem I complained about. Default collection names are working for me in 4.3 for query and update requests.

          Show
          Jack Krupansky added a comment - This is a branch_4x issue, but not a 4.3 issue - if it's the same problem I complained about. Default collection names are working for me in 4.3 for query and update requests.
          Hide
          Hoss Man added a comment -

          Updated patch to correct a test assumption.

          Jack: please note the issue description – this affects any use of solr with the new style solr.xml, ie "core discovery" mode.

          this is the default on the 4x branch example so it's very obvious, but core discovery mode was a feature included in 4.3 – but the 4.3 example configs did not use it by default.

          Show
          Hoss Man added a comment - Updated patch to correct a test assumption. Jack: please note the issue description – this affects any use of solr with the new style solr.xml, ie "core discovery" mode. this is the default on the 4x branch example so it's very obvious, but core discovery mode was a feature included in 4.3 – but the 4.3 example configs did not use it by default.
          Hide
          Hoss Man added a comment -

          Sigh ... the actual updated patch file this time.

          Show
          Hoss Man added a comment - Sigh ... the actual updated patch file this time.
          Hide
          Commit Tag Bot added a comment -

          [trunk commit] hossman
          http://svn.apache.org/viewvc?view=revision&revision=1480515

          SOLR-4803: Fixed core discovery mode (ie: new style solr.xml) to treat 'collection1' as the default core name

          Show
          Commit Tag Bot added a comment - [trunk commit] hossman http://svn.apache.org/viewvc?view=revision&revision=1480515 SOLR-4803 : Fixed core discovery mode (ie: new style solr.xml) to treat 'collection1' as the default core name
          Hide
          Commit Tag Bot added a comment -

          [branch_4x commit] hossman
          http://svn.apache.org/viewvc?view=revision&revision=1480517

          SOLR-4803: Fixed core discovery mode (ie: new style solr.xml) to treat 'collection1' as the default core name (merge r1480515)

          Show
          Commit Tag Bot added a comment - [branch_4x commit] hossman http://svn.apache.org/viewvc?view=revision&revision=1480517 SOLR-4803 : Fixed core discovery mode (ie: new style solr.xml) to treat 'collection1' as the default core name (merge r1480515)
          Hide
          Hoss Man added a comment -

          Committed revision 1480515.
          Committed revision 1480517.

          Show
          Hoss Man added a comment - Committed revision 1480515. Committed revision 1480517.
          Hide
          Mark Miller added a comment -

          This is a 4.3 issue to some small degree - if you chose to use the new style solr.xml stuff, you had no option for a default core. This wouldn't have affected any upgraders though, and the new style solr.xml stuff has enough problems (you cant use the right data dir specification for solrcloud, core properties are not persisted - a problem for std Solr, broken SolrCloud, a lot of other little bugs) that I'd say you should not use it in 4.3 and I can't see how it will be solid until 4.4. There has already been a lot of work and there is a fair amount more to come, and it's enough churn to make a backport scary - I think the only way is forward.

          What prompted finding this issue with the smoke tests was shifting the example to use the new style so that we are forced to make it solid ASAP. I'll be contributing some more fixes shortly myself.

          So this issue dones't really contribute to a 4.3.1 release IMO.

          Show
          Mark Miller added a comment - This is a 4.3 issue to some small degree - if you chose to use the new style solr.xml stuff, you had no option for a default core. This wouldn't have affected any upgraders though, and the new style solr.xml stuff has enough problems (you cant use the right data dir specification for solrcloud, core properties are not persisted - a problem for std Solr, broken SolrCloud, a lot of other little bugs) that I'd say you should not use it in 4.3 and I can't see how it will be solid until 4.4. There has already been a lot of work and there is a fair amount more to come, and it's enough churn to make a backport scary - I think the only way is forward. What prompted finding this issue with the smoke tests was shifting the example to use the new style so that we are forced to make it solid ASAP. I'll be contributing some more fixes shortly myself. So this issue dones't really contribute to a 4.3.1 release IMO.
          Hide
          Jan Høydahl added a comment -

          Agree that this would not bite people upgrading, so it's not urgent. Perhaps a Good Thing™ would be to add some errata info to http://lucene.apache.org/solr/4_3_0/ to warn against using the new solr.xml format until 4.4?

          Show
          Jan Høydahl added a comment - Agree that this would not bite people upgrading, so it's not urgent. Perhaps a Good Thing™ would be to add some errata info to http://lucene.apache.org/solr/4_3_0/ to warn against using the new solr.xml format until 4.4?
          Hide
          Steve Rowe added a comment -

          Bulk close resolved 4.4 issues

          Show
          Steve Rowe added a comment - Bulk close resolved 4.4 issues

            People

            • Assignee:
              Hoss Man
              Reporter:
              Hoss Man
            • Votes:
              1 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development