Solr
  1. Solr
  2. SOLR-465

Add configurable DirectoryProvider so that alternate Directory implementations can be specified via solrconfig.xml

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 1.4
    • Component/s: None
    • Labels:
      None

      Description

      Solr is presently hard-coded to use the FSDirectory implementation in Lucene. Other Directory implementations are possible. This patch creates a new DirectoryProvider interface and extends SolrCore to load an implementation of it from solrconfig.xml (if specified). If not specified, then it will fallback to the FSDirectory.

      A DirectoryProvider plugin can be configured in solrconfig.xml with the following XML:
      <directoryProvider class="class.name">
      <!-- Parameters as required by the implementation -->
      </directoryProvider>

      This patch was created against solr trunk checked out on 11/20/2007. Most of it is new code and should apply cleanly or with minor relocation. If it does not, let me know and I will update.

      1. solr-directory-provider.patch
        13 kB
        TJ Laurenzo
      2. SOLR-465-fixes.patch
        2 kB
        Andrey Klochkov
      3. SOLR-465.patch
        31 kB
        Mark Miller
      4. SOLR-465.patch
        30 kB
        Mark Miller
      5. SOLR-465.patch
        0.8 kB
        Mark Miller
      6. SOLR-465.patch
        1 kB
        Mark Miller
      7. SOLR-465.patch
        2 kB
        Mark Miller
      8. SOLR-465.patch
        0.7 kB
        Mark Miller

        Issue Links

          Activity

          Hide
          Uwe Schindler added a comment -

          I committed a changed version of the FSDirectory today to Lucene trunk (LUCENE-1658). The ctor/class name of the default directory impl is now SimpleFSDirectory vs. NIOFSDirectory and MMapDirectory. All three dirs can no instantiated by a one-arg and two-arg ctor (File[, LockFactory]).

          FSDirectory will get abstract in 3.0 and so cannot be instantiated (only protected ctor). It is now also a factory, to choose automatically the best variant for the platform: FSDirectory.open(File[, LockFactory])

          Show
          Uwe Schindler added a comment - I committed a changed version of the FSDirectory today to Lucene trunk ( LUCENE-1658 ). The ctor/class name of the default directory impl is now SimpleFSDirectory vs. NIOFSDirectory and MMapDirectory. All three dirs can no instantiated by a one-arg and two-arg ctor (File [, LockFactory] ). FSDirectory will get abstract in 3.0 and so cannot be instantiated (only protected ctor). It is now also a factory, to choose automatically the best variant for the platform: FSDirectory.open(File [, LockFactory] )
          Hide
          Mark Miller added a comment -

          I should have also changed the SolrCore use of SolrIndexWriter to create an index to use a non deprecated SolrIndexWriter constructor.

          Show
          Mark Miller added a comment - I should have also changed the SolrCore use of SolrIndexWriter to create an index to use a non deprecated SolrIndexWriter constructor.
          Hide
          Yonik Seeley added a comment -

          Thanks Mark, committed.

          Show
          Yonik Seeley added a comment - Thanks Mark, committed.
          Hide
          Mark Miller added a comment -

          Sorry, missed UpdateHandler in that last patch - new patch to fix. Eclipse is both a blessing and a curse

          Show
          Mark Miller added a comment - Sorry, missed UpdateHandler in that last patch - new patch to fix. Eclipse is both a blessing and a curse
          Hide
          Mark Miller added a comment -

          changes made

          Show
          Mark Miller added a comment - changes made
          Hide
          Mark Miller added a comment -

          Bah, those constructors came in after this patch - they should be the issues reopened!

          I'll put up a patch changing it.

          Show
          Mark Miller added a comment - Bah, those constructors came in after this patch - they should be the issues reopened! I'll put up a patch changing it.
          Hide
          Hoss Man added a comment -

          Hmmm... yes, this does seem to be an oversight.

          the root issue seems to be that there is still a SolrIndexWriter constructor that takes a IndexDeletionPolicy but does not take a DirectoryFactory.

          the latest patch seems to be adding yet another constructor (that takes both) and modifies the callers of the existing IndexDeletionPolicy method to use the new one ... but since the IndexDeletionPolicy code was added after 1.3 we can just change it.

          It also seems like it might be a good idea to add some logging to SolrIndexWriter.getDirectory(String,SolrIndexConfig) warning people when legacy code is being used that might not be respecting their DirectoryFactory.

          reopening, assigning to Yonik to review since he & Miller worked on this originally

          Show
          Hoss Man added a comment - Hmmm... yes, this does seem to be an oversight. the root issue seems to be that there is still a SolrIndexWriter constructor that takes a IndexDeletionPolicy but does not take a DirectoryFactory. the latest patch seems to be adding yet another constructor (that takes both) and modifies the callers of the existing IndexDeletionPolicy method to use the new one ... but since the IndexDeletionPolicy code was added after 1.3 we can just change it. It also seems like it might be a good idea to add some logging to SolrIndexWriter.getDirectory(String,SolrIndexConfig) warning people when legacy code is being used that might not be respecting their DirectoryFactory. reopening, assigning to Yonik to review since he & Miller worked on this originally
          Hide
          Andrey Klochkov added a comment -

          I tried to use custom Directory implementation which doesn't use file system and found that there are some places in the Solr code which don't let to do this 'cause they instantiate FSDirectory. Here is my patch, please review it and apply if it's Ok. I don't have rights to re-open the issue.

          Show
          Andrey Klochkov added a comment - I tried to use custom Directory implementation which doesn't use file system and found that there are some places in the Solr code which don't let to do this 'cause they instantiate FSDirectory. Here is my patch, please review it and apply if it's Ok. I don't have rights to re-open the issue.
          Hide
          Yonik Seeley added a comment -

          Committed. Thanks Mark!

          Show
          Yonik Seeley added a comment - Committed. Thanks Mark!
          Hide
          Mark Miller added a comment -

          Sorry, this fell off my radar. I'll post another patch to finish this up this weekend.

          Show
          Mark Miller added a comment - Sorry, this fell off my radar. I'll post another patch to finish this up this weekend.
          Hide
          Shalin Shekhar Mangar added a comment -

          I have upgraded Lucene jars to r719351

          Show
          Shalin Shekhar Mangar added a comment - I have upgraded Lucene jars to r719351
          Hide
          Mark Miller added a comment -

          We will need a tiny new patch as well...ill put it up as soon as the jars are upgraded .

          Show
          Mark Miller added a comment - We will need a tiny new patch as well...ill put it up as soon as the jars are upgraded .
          Hide
          Shalin Shekhar Mangar added a comment -

          LUCENE-1451 has been committed. So all we need is to upgrade the lucene jars in the solr lib, right?

          If there are no objections, I'll do that today.

          Show
          Shalin Shekhar Mangar added a comment - LUCENE-1451 has been committed. So all we need is to upgrade the lucene jars in the solr lib, right? If there are no objections, I'll do that today.
          Hide
          Mark Miller added a comment -

          Thats the hope once we upgrade to a Lucene with LUCENE-1451, but right now its not possible to auto do it - you have to set the system property.

          Show
          Mark Miller added a comment - Thats the hope once we upgrade to a Lucene with LUCENE-1451 , but right now its not possible to auto do it - you have to set the system property.
          Hide
          Shalin Shekhar Mangar added a comment -

          Possibly...I think automatic has a lot of allure though...

          Automatic is great. I was only referring to using solrconfig.xml instead of system property for this. I see now that this system property is used by Lucene and not Solr.

          Just for my understanding, now Solr uses NIOFSDirectory automatically but only on non-windows platforms, is that correct?

          Show
          Shalin Shekhar Mangar added a comment - Possibly...I think automatic has a lot of allure though... Automatic is great. I was only referring to using solrconfig.xml instead of system property for this. I see now that this system property is used by Lucene and not Solr. Just for my understanding, now Solr uses NIOFSDirectory automatically but only on non-windows platforms, is that correct?
          Hide
          Yonik Seeley added a comment -

          reopening. LUCENE-1451 has been committed, adding public constructors to FSDirectory and subclasses.

          Show
          Yonik Seeley added a comment - reopening. LUCENE-1451 has been committed, adding public constructors to FSDirectory and subclasses.
          Hide
          Mark Miller added a comment -

          But couldn't one start the container with the appropriate system property pointing to the new NIOFSDirectory class and "turn on" NIOFSDirectory (or any other Directory) that way?

          Yes. Thats why I say its not really broken. It will still work, and you can still change the impl...but the code implies you will get the right impl for the right OS, and you won't without using that system property. Hoping to make that pain go away.

          Why not specify in solrconfig.xml?

          Possibly...I think automatic has a lot of allure though...

          Is it possible to use RAMDirectory too?

          The factory returns a Directory, so it is in that sense...it may have other ramifications though depending on what solr assumes. Don't know off hand.

          Show
          Mark Miller added a comment - But couldn't one start the container with the appropriate system property pointing to the new NIOFSDirectory class and "turn on" NIOFSDirectory (or any other Directory) that way? Yes. Thats why I say its not really broken. It will still work, and you can still change the impl...but the code implies you will get the right impl for the right OS, and you won't without using that system property. Hoping to make that pain go away. Why not specify in solrconfig.xml? Possibly...I think automatic has a lot of allure though... Is it possible to use RAMDirectory too? The factory returns a Directory, so it is in that sense...it may have other ramifications though depending on what solr assumes. Don't know off hand.
          Hide
          Shalin Shekhar Mangar added a comment -

          But couldn't one start the container with the appropriate system property pointing to the new NIOFSDirectory class and "turn on" NIOFSDirectory (or any other Directory) that way?

          Why not specify in solrconfig.xml?

          Another question I had was – Is it possible to use RAMDirectory too?

          Show
          Shalin Shekhar Mangar added a comment - But couldn't one start the container with the appropriate system property pointing to the new NIOFSDirectory class and "turn on" NIOFSDirectory (or any other Directory) that way? Why not specify in solrconfig.xml? Another question I had was – Is it possible to use RAMDirectory too?
          Hide
          Otis Gospodnetic added a comment -

          But couldn't one start the container with the appropriate system property pointing to the new NIOFSDirectory class and "turn on" NIOFSDirectory (or any other Directory) that way?

          Show
          Otis Gospodnetic added a comment - But couldn't one start the container with the appropriate system property pointing to the new NIOFSDirectory class and "turn on" NIOFSDirectory (or any other Directory) that way?
          Hide
          Mark Miller added a comment -

          You will get an FSDirectory no matter which OS. Open is a static method that pulls from a system property for the directory impl to use. So its not broken per say (you can set the property and get the right stuff), but you wont get the NIOFSDirectory automatically on non windows like the code implies. I'm going to try and make it workable at the Lucene level without the sys property is my hope.

          Show
          Mark Miller added a comment - You will get an FSDirectory no matter which OS. Open is a static method that pulls from a system property for the directory impl to use. So its not broken per say (you can set the property and get the right stuff), but you wont get the NIOFSDirectory automatically on non windows like the code implies. I'm going to try and make it workable at the Lucene level without the sys property is my hope.
          Hide
          Yonik Seeley added a comment -

          Hmmm, can you expand on what's wrong?

          Show
          Yonik Seeley added a comment - Hmmm, can you expand on what's wrong?
          Hide
          Mark Miller added a comment -

          Sorry guys, this patch isnt right for NIODirectory. You can't control the directory that way. Will have to think of something else...at worst users will have to select with a java property rather than auto getting the good stuff.

          Show
          Mark Miller added a comment - Sorry guys, this patch isnt right for NIODirectory. You can't control the directory that way. Will have to think of something else...at worst users will have to select with a java property rather than auto getting the good stuff.
          Hide
          Yonik Seeley added a comment -

          Thanks guys, I just committed this.

          Show
          Yonik Seeley added a comment - Thanks guys, I just committed this.
          Hide
          Yonik Seeley added a comment -

          Patch looks good to me - I'll commit shortly.

          Show
          Yonik Seeley added a comment - Patch looks good to me - I'll commit shortly.
          Hide
          Mark Miller added a comment -

          Updated to trunk and now uses NIOFSDirectory if non-windows OS is detected.

          Show
          Mark Miller added a comment - Updated to trunk and now uses NIOFSDirectory if non-windows OS is detected.
          Hide
          Mark Miller added a comment -

          I've worked a bit on updating this to trunk, but it has a bit of overlap with solr-243, so if either are going in, it will be much easier to finish the other after.

          Show
          Mark Miller added a comment - I've worked a bit on updating this to trunk, but it has a bit of overlap with solr-243, so if either are going in, it will be much easier to finish the other after.
          Hide
          Mark Miller added a comment -

          New patch.

          Updates things to trunk, tweaks things to be more like latest SOLR-243, adds a unit test to make sure alternate DirectoryFactory is used.

          Looks like we will want to use an alternate Directory for UNIX systems due to LUCENE-753 (which in the end adds a new Directory implementation called org.apache.lucene.store.NIOFSDirectory). Makes sense to fit this issue into that.

          Once Lucene is up to date, I'll add a new patch that returns a directory based on the OS from the default Factory.

          • Mark
          Show
          Mark Miller added a comment - New patch. Updates things to trunk, tweaks things to be more like latest SOLR-243 , adds a unit test to make sure alternate DirectoryFactory is used. Looks like we will want to use an alternate Directory for UNIX systems due to LUCENE-753 (which in the end adds a new Directory implementation called org.apache.lucene.store.NIOFSDirectory). Makes sense to fit this issue into that. Once Lucene is up to date, I'll add a new patch that returns a directory based on the OS from the default Factory. Mark
          Hide
          Hoss Man added a comment -

          i haven't looked at the patch, but i just wanted to point out that SOLR-465 and SOLR-243 should be dealt with holistically so that the solutions to each of the use cases work well together.

          (ie: perhaps DirectoryProvider should be a property of an IndexReaderFactory)

          Show
          Hoss Man added a comment - i haven't looked at the patch, but i just wanted to point out that SOLR-465 and SOLR-243 should be dealt with holistically so that the solutions to each of the use cases work well together. (ie: perhaps DirectoryProvider should be a property of an IndexReaderFactory)
          Hide
          TJ Laurenzo added a comment -

          Patch to add DirectoryProvider configurability to Solr.

          Show
          TJ Laurenzo added a comment - Patch to add DirectoryProvider configurability to Solr.

            People

            • Assignee:
              Yonik Seeley
              Reporter:
              TJ Laurenzo
            • Votes:
              3 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 0.25h
                0.25h
                Remaining:
                Remaining Estimate - 0.25h
                0.25h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development