Solr
  1. Solr
  2. SOLR-6338

coreRootDirectory requires trailing slash, or SolrCloud cores are created in wrong location

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.9
    • Fix Version/s: 4.10, 6.0
    • Component/s: None
    • Labels:
    • Environment:

      x86_64 Linux
      x86_64 Sun Java 1.7.0_51

      Description

      If the coreRootDirectory setting does not end with a filesystem path separator, cores are not created in the correct location.

      e.g.
      coreRootDirectory=/usr/local/solr/cores

      For a collection xyz, API creates '/usr/local/solr/coresxyz_shardN_replicaN' config directory instead of '/usr/local/solr/cores/xyz_shardN_replicaN'

      Adding a trailing slash to coreRootDirectory is a viable workaround:

      coreRootDirectory=/usr/local/solr/cores/

      1. SOLR-6338.patch
        3 kB
        Erick Erickson

        Activity

        Hide
        Erick Erickson added a comment - - edited

        Ugggh. I really, really hate those kinds of sensitivities.

        Nice catch!

        I'll assign it to myself, but feel free to take it if you want it I've got about 11 other things on my plate.

        But I really don't want to lose track of it.

        Show
        Erick Erickson added a comment - - edited Ugggh. I really, really hate those kinds of sensitivities. Nice catch! I'll assign it to myself, but feel free to take it if you want it I've got about 11 other things on my plate. But I really don't want to lose track of it.
        Hide
        Shawn Heisey added a comment -

        I did look at the code and couldn't decipher it to figure out how to fix the issue. I would have no problem taking the issue and working on it, if I had any idea what to do!

        Show
        Shawn Heisey added a comment - I did look at the code and couldn't decipher it to figure out how to fix the issue. I would have no problem taking the issue and working on it, if I had any idea what to do!
        Hide
        Primož Skale added a comment -

        I think that only one line of code needs to be changed, namely:

        ConfigSolr.java
        107  public String getCoreRootDirectory() {
        108    get(CfgProp.SOLR_COREROOTDIRECTORY, config.getResourceLoader().getInstanceDir())
        109  }
        

        to

        ConfigSolr.java
        107  public String getCoreRootDirectory() {
        108    return SolrResourceLoader.normalizeDir( get(CfgProp.SOLR_COREROOTDIRECTORY, config.getResourceLoader().getInstanceDir()) );
        109  }
        

        but the test TestSolrXml.java fails in testPropertySub() and testAllInfoPresent().

        I have a hard time deciding if this is a problem of a test or of a code. If I hand-test it, it works (eg. it creates the core in correct location) if coreRootDirectory property has no trailing slash in solr.xml.

        Show
        Primož Skale added a comment - I think that only one line of code needs to be changed, namely: ConfigSolr.java 107 public String getCoreRootDirectory() { 108 get(CfgProp.SOLR_COREROOTDIRECTORY, config.getResourceLoader().getInstanceDir()) 109 } to ConfigSolr.java 107 public String getCoreRootDirectory() { 108 return SolrResourceLoader.normalizeDir( get(CfgProp.SOLR_COREROOTDIRECTORY, config.getResourceLoader().getInstanceDir()) ); 109 } but the test TestSolrXml.java fails in testPropertySub() and testAllInfoPresent() . I have a hard time deciding if this is a problem of a test or of a code. If I hand-test it, it works (eg. it creates the core in correct location) if coreRootDirectory property has no trailing slash in solr.xml.
        Hide
        Erick Erickson added a comment - - edited

        All of the test failures in TestSolrXml are some form like:
        assertEquals("core root dir", "testCoreRootDirectory", cfg.getCoreRootDirectory());

        And can be fixed by adding the trailing slash to the hard-coded string like this:
        assertEquals("core root dir", "testCoreRootDirectory/", cfg.getCoreRootDirectory());

        So the failures are consistent with the changes to the code, at least at a glance.
        I suppose I have to make sure the trailing slash works on Windows for the
        tests though.

        Now, I've just glanced at the code change, and it seems fine to me. I don't think
        there's any problem with changing the tests to have the trailing slash.

        So if anyone wants to weigh in with whether the change has any problems please do.
        Otherwise it seems like we should commit it. I'm running the full test suite
        now and unless someone objects I'll commit this shortly.

        Show
        Erick Erickson added a comment - - edited All of the test failures in TestSolrXml are some form like: assertEquals("core root dir", "testCoreRootDirectory", cfg.getCoreRootDirectory()); And can be fixed by adding the trailing slash to the hard-coded string like this: assertEquals("core root dir", "testCoreRootDirectory/", cfg.getCoreRootDirectory()); So the failures are consistent with the changes to the code, at least at a glance. I suppose I have to make sure the trailing slash works on Windows for the tests though. Now, I've just glanced at the code change, and it seems fine to me. I don't think there's any problem with changing the tests to have the trailing slash. So if anyone wants to weigh in with whether the change has any problems please do. Otherwise it seems like we should commit it. I'm running the full test suite now and unless someone objects I'll commit this shortly.
        Hide
        ASF subversion and git services added a comment -

        Commit 1618705 from Erick Erickson in branch 'dev/trunk'
        [ https://svn.apache.org/r1618705 ]

        SOLR-6338: coreRootDirectory requires trailing slash, or SolrCloud cores are created in wrong location. Thanks Primoz

        Show
        ASF subversion and git services added a comment - Commit 1618705 from Erick Erickson in branch 'dev/trunk' [ https://svn.apache.org/r1618705 ] SOLR-6338 : coreRootDirectory requires trailing slash, or SolrCloud cores are created in wrong location. Thanks Primoz
        Hide
        ASF subversion and git services added a comment -

        Commit 1618717 from Erick Erickson in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1618717 ]

        SOLR-6338: coreRootDirectory requires trailing slash, or SolrCloud cores are created in wrong location

        Show
        ASF subversion and git services added a comment - Commit 1618717 from Erick Erickson in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1618717 ] SOLR-6338 : coreRootDirectory requires trailing slash, or SolrCloud cores are created in wrong location
        Hide
        Erick Erickson added a comment -

        Thanks Primož!

        Show
        Erick Erickson added a comment - Thanks Primož!
        Hide
        Erick Erickson added a comment -

        Forgot patch

        Show
        Erick Erickson added a comment - Forgot patch
        Hide
        Erick Erickson added a comment -

        Forgot to attach patch.... Sorry for the noise.

        Show
        Erick Erickson added a comment - Forgot to attach patch.... Sorry for the noise.
        Hide
        Erick Erickson added a comment -

        Attached patch

        Show
        Erick Erickson added a comment - Attached patch

          People

          • Assignee:
            Erick Erickson
            Reporter:
            Virender Khatri
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development